Bug 22033 - [GTK] CTI/Linux r38064 crashes; JIT requires executable memory
Summary: [GTK] CTI/Linux r38064 crashes; JIT requires executable memory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Critical
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-02 01:17 PST by Martin Sourada
Modified: 2008-11-06 10:48 PST (History)
4 users (show)

See Also:


Attachments
backtrace from gtklauncher (8.51 KB, text/plain)
2008-11-02 01:21 PST, Martin Sourada
no flags Details
config.log (with JIT enable) r38078 - Fedora 9 (45.16 KB, text/plain)
2008-11-03 14:27 PST, Audu Jerome
no flags Details
config.log, Fedora 10 (rawhide), built as a rpm package, r38064 (58.09 KB, text/plain)
2008-11-03 15:13 PST, Martin Sourada
no flags Details
Use PROT_EXEC to fix JIT crash on Fedora (1.29 KB, patch)
2008-11-04 04:55 PST, Alp Toker
no flags Details | Formatted Diff | Diff
Alternative fix (1.73 KB, patch)
2008-11-04 05:41 PST, Alp Toker
no flags Details | Formatted Diff | Diff
Fix CTI on affected systems (2.74 KB, patch)
2008-11-06 09:38 PST, Alp Toker
zwarich: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sourada 2008-11-02 01:17:08 PST
r38064 gtk build crashes when loading google. This a regression since r37790. I'll attach a backtrace from GTKLauncher using gdb.
Comment 1 Martin Sourada 2008-11-02 01:21:22 PST
Created attachment 24839 [details]
backtrace from gtklauncher
Comment 2 Alp Toker 2008-11-03 09:48:36 PST
Martin,

Looks like this issue is caused by the JavaScript JIT engine that was enabled on Linux in r37996.

You should be able to avoid the crash by disabling the JIT passing --disable-jit to the configure script, but it's important that we fix this in SVN:

Before trying --disable-jit can you attach the 'config.log' file to this bug and report your distribution/version, CPU architecture (and if you known of any kernel security extensions you have enabled) here?
Comment 3 Audu Jerome 2008-11-03 14:27:32 PST
Created attachment 24867 [details]
config.log (with JIT enable) r38078 - Fedora 9
Comment 4 Audu Jerome 2008-11-03 14:29:06 PST
(In reply to comment #2)
> Looks like this issue is caused by the JavaScript JIT engine that was enabled
> on Linux in r37996.

Disable JIT engine avoid crash. (Fedora 8 (gcc-4.1.2) & Fedora 9 (gcc-4.3.0))

> 
> You should be able to avoid the crash by disabling the JIT passing
> --disable-jit to the configure script, but it's important that we fix this in
> SVN:
> 
> Before trying --disable-jit can you attach the 'config.log' file to this bug
> and report your distribution/version, CPU architecture (and if you known of any
> kernel security extensions you have enabled) here?
> 
Fedora 9
System: Linux 2.6.26.7-86.fc9.i686 #1 SMP Sat Oct 25 21:03:54 CEST 2008 i686
selinux=disable
model name	: AMD Athlon(tm) XP 2600+

Fedora 8
System: kernel-2.6.26.7-54.fc8
selinux=disable
model name	:Intel Xeon (dual core)



Comment 5 Martin Sourada 2008-11-03 15:13:51 PST
Created attachment 24872 [details]
config.log, Fedora 10 (rawhide), built as a rpm package, r38064

I've noticed it happens also on other pages.

$ uname -a
Linux pc-notebook 2.6.27.4-68.fc10.i686 #1 SMP Thu Oct 30 00:49:42 EDT 2008 i686 i686 i386 GNU/Linux
selinux=permisive
model name: Intel Celeron M420

I'm on Fedora 10 (rawhide), I've built and installed WebKit-gtk as a rpm package.
Comment 6 Alp Toker 2008-11-03 17:45:06 PST
I'm not able to try this on Fedora right now.

Please try building with the JIT enabled and without these extra CFLAGS/CXXFLAGS that config.log reports:

-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -fasynchronous-unwind-tables

Knowing whether one of these compiler flags is breaking the JIT will help narrow down the issue and work out a solution (either removing that flag from your build configuration or fixing the JIT to work with it).

If the problem doesn't lie with these extra flags you're using on Fedora, it might be down to executable memory protection features (NX?) being enabled on Fedora which isn't too difficult to fix. I'm not familiar with the security features enabled in Fedora by default unfortunately.
Comment 7 Alp Toker 2008-11-04 04:24:50 PST
OK, I have a reproduction of the issue here. It's not the compiler flags but ExecShield. Can you confirm that this fixes GtkLauncher with the JIT enabled?

# sysctl -w kernel.exec-shield=0
Comment 8 Alp Toker 2008-11-04 04:55:44 PST
Created attachment 24884 [details]
Use PROT_EXEC to fix JIT crash on Fedora

This patch gets the JIT working on Fedora with ExecShield/NX enabled.

It'd be helpful if you could confirm that this works for you, but please don't ship it/package it just yet (I'm going to see if we can do this in a slightly better way).
Comment 9 Audu Jerome 2008-11-04 05:38:07 PST
(In reply to comment #8)
> Use PROT_EXEC to fix JIT crash on Fedora
> This patch gets the JIT working on Fedora with ExecShield/NX enabled.
> It'd be helpful if you could confirm that this works for you, but please don't
> ship it/package it just yet (I'm going to see if we can do this in a slightly
> better way).

Patch is working for me on Fedora 9 - kernel-2.6.26.6-49.fc8PAE / Intel(R) Xeon(R) CPU using Webkit-r38068

But, without patch, "sysctl -w kernel.exec-shield=0" doesn't help.
Comment 10 Alp Toker 2008-11-04 05:41:50 PST
Created attachment 24885 [details]
Alternative fix

This patch only marks memory executable when necessary. It's a bit of a hack since it wastes a lot of memory but works just the same. I wouldn't advise shipping this patch at all for that reason.

Have CC'd FastMalloc engineer Mark Rowe who can advise on how to proceed. (We'll probably want FastMalloc to allocate out of two separate memory pools as necessary eventually.)
Comment 11 Audu Jerome 2008-11-04 05:52:55 PST
(In reply to comment #10)
> Created an attachment (id=24885) [edit]
> Alternative fix
> This patch only marks memory executable when necessary. It's a bit of a hack
> since it wastes a lot of memory but works just the same. I wouldn't advise
> shipping this patch at all for that reason.

Second patch (alone) is working for me on Fedora 9 - kernel-2.6.26.6-49.fc8PAE / Intel(R) Xeon(R) CPU using Webkit-r38068


Comment 12 Alp Toker 2008-11-04 06:00:39 PST
(In reply to comment #9)
> (In reply to comment #8)
> > Use PROT_EXEC to fix JIT crash on Fedora
> > This patch gets the JIT working on Fedora with ExecShield/NX enabled.
> > It'd be helpful if you could confirm that this works for you, but please don't
> > ship it/package it just yet (I'm going to see if we can do this in a slightly
> > better way).
> 
> Patch is working for me on Fedora 9 - kernel-2.6.26.6-49.fc8PAE / Intel(R)
> Xeon(R) CPU using Webkit-r38068
> 
> But, without patch, "sysctl -w kernel.exec-shield=0" doesn't help.
> 

Without knowing much about the Fedora setup, it looks like your hardware (Xeon) supports genuine NX so even when you disable software exec-shield, the CPU security functionality is still active. I've been testing on a plain old x86 VM so kernel.exec-shield=0 was enough for me to track down the issue.
Comment 13 Martin Sourada 2008-11-04 13:20:22 PST
(In reply to comment #7)
> OK, I have a reproduction of the issue here. It's not the compiler flags but
> ExecShield. Can you confirm that this fixes GtkLauncher with the JIT enabled?
> 
> # sysctl -w kernel.exec-shield=0
> 

Yeah, this fixes it for me on F10 (tested both with enabled and disabled, disabled works, enabled leads to instant crash). I don't have enough disk space ATM to build webkit from sources to test it with one of the patch you attached, but as soon as I free some, I'll give it a shot.
Comment 14 Alp Toker 2008-11-06 09:38:57 PST
Created attachment 24945 [details]
Fix CTI on affected systems
Comment 15 Alp Toker 2008-11-06 10:31:21 PST
Comment on attachment 24945 [details]
Fix CTI on affected systems

>Index: JavaScriptCore/ChangeLog
>===================================================================
>--- JavaScriptCore/ChangeLog	(revision 38170)
>+++ JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,23 @@
>+2008-11-06  Alp Toker  <alp@nuanti.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=22033
>+        [GTK] CTI/Linux r38064 crashes; JIT requires executable memory
>+
>+        Mark pages allocated by the FastMalloc mmap code path executable with
>+        PROT_EXEC. This fixes crashes seen on CPUs and kernels that enforce
>+        non-executable memory (like ExecShield on Fedora Linux) when the JIT
>+        is enabled.
>+
>+        This patch does not resolve the issue on debug builds so affected
>+        developers may still need to pass --disable-jit to configure.
>+
>+        * wtf/TCSystemAlloc.cpp:
>+        (TryMmap):
>+        (TryDevMem):
>+        (TCMalloc_SystemRelease):
>+
> 2008-11-06  Kristian Amlie  <kristian.amlie@nokia.com>
> 
>         Reviewed by Simon Hausmann.
>Index: JavaScriptCore/wtf/TCSystemAlloc.cpp
>===================================================================
>--- JavaScriptCore/wtf/TCSystemAlloc.cpp	(revision 38170)
>+++ JavaScriptCore/wtf/TCSystemAlloc.cpp	(working copy)
>@@ -51,6 +51,14 @@
> #include "TCSpinLock.h"
> #include "UnusedParam.h"
> 
>+#if HAVE(MMAP)
>+static const int cProtFlags = PROT_READ | PROT_WRITE
>+#if ENABLE(CTI)

^ Will make this #if ENABLE(CTI) && PLATFORM(GTK) as requested.
Comment 16 Cameron Zwarich (cpst) 2008-11-06 10:42:32 PST
Comment on attachment 24945 [details]
Fix CTI on affected systems

r=me if you make this GTK-only like promised.
Comment 17 Alp Toker 2008-11-06 10:48:33 PST
Landed in r38187. Thanks Cameron!