RFR (S) JDK-7107135 - Stack guard pages becomes writable
Ioi Lam
ioi.lam at oracle.com
Tue Mar 5 11:03:08 PST 2013
Hi,
I have updated the patch to be based on hotspot-rt repo:
http://cr.openjdk.java.net/~iklam/7107135/stack_guard_003/
<http://cr.openjdk.java.net/%7Eiklam/7107135/stack_guard_003/>
The only code change from the previous patch is in os::dll_load() in
os_linux.cpp, based on Zhengyu's feedback
* Try to fix the stack guard only if the current thread is a java
thread, and its state is_thread_in_native. This would be the case if
we're called from System.loadLibrary
* For other cases, (such as compiler thread loading hsdis-<arch>.so),
sometimes it's not possible to enter safe point (I'd get some sort
of "out of order -- possible deadlock" asserts). Since this is not
typical usage, I'll not try to fix the stack guard.
Thanks
- Ioi
On 03/01/2013 09:50 AM, Ioi Lam wrote:
> Volker,
>
> You're right. I am not very familiar with the process. Thanks for
> pointing this out.
>
> I will test the code on hsx/hotspot-rt and will post a new webrev.
>
> Thanks
> - Ioi
>
> On 03/01/2013 02:04 AM, Volker Simonis wrote:
>> On Thu, Feb 28, 2013 at 9:51 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>> Hi,
>>>
>>> Could someone give an official review for this bug so I could get it
>>> committed (to hsx24)?
>> Why hsx24?
>> Shouldn't this go to hsx/hotspot-rt and then be back-ported to hsx24?
>>
>> Regards,
>> Volker
>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>>
>>> On 02/27/2013 09:25 PM, Ioi Lam wrote:
>>>> Sorry, here it is:
>>>>
>>>> http://cr.openjdk.java.net/~iklam/7107135/stack_guard_002/
>>>>
>>>> - Ioi
>>>>
>>>>
>>>> On 02/27/2013 08:14 PM, David Holmes wrote:
>>>>> On 28/02/2013 12:31 PM, Ioi Lam wrote:
>>>>>> I have updated the patch:
>>>>>>
>>>>>> http://javaweb.us.oracle.com/~iklam/webrev/7107135/stack_guard_002/
>>>>>
>>>>> Please put the webrev on cr.openjdk.java.net
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> The only changes since the last version are in os_linux.cpp,
>>>>>> os_linux_x86.cpp
>>>>>> and os_linux_sparc.cpp.
>>>>>>
>>>>>> 1. Added assert in (is_init_completed()==false) case that no Java
>>>>>> threads have
>>>>>> been created (line 1867 os_linux.cpp)
>>>>>>
>>>>>> 2. Avoid unnecessary condition checks in os::Linux::dll_load_inner()
>>>>>>
>>>>>> 3. Print a diagnostic message when we crash the native stack to
>>>>>> suggest
>>>>>> the user
>>>>>> check for executable stack .so files.
>>>>>>
>>>>>> I also restored the SAP copyright in the test sources.
>>>>>>
>>>>>> - Ioi
>>>>>>
>>>>>>
>>>>>> On 02/27/2013 03:03 PM, Ioi Lam wrote:
>>>>>>> On 02/21/2013 07:07 PM, David Holmes wrote:
>>>>>>>> <fixed the subject to try and get the email thread to flow >
>>>>>>>>
>>>>>>>> On 22/02/2013 1:29 AM, Zhengyu Gu wrote:
>>>>>>>>> Hi Ioi,
>>>>>>>>>
>>>>>>>>> 1. os_linux.cpp #1877 : this will restrict os::dll_load()
>>>>>>>>> caller to
>>>>>>>>> Java
>>>>>>>>> thread only, is it ok?
>>>>>>>>
>>>>>>>> That bit doesn't make sense to me:
>>>>>>>>
>>>>>>>> 1877 ThreadInVMfromNative tiv(JavaThread::current());
>>>>>>>> 1878 debug_only(VMNativeEntryWrapper vew;)
>>>>>>>>
>>>>>>>> shouldn't we already be _thread_in_vm at this point ??
>>>>>>>>
>>>>>>> I tried comment them out, and I get an abort. The thread state is
>>>>>>> "_thread_in_native". So it looks like we need this code to make
>>>>>>> the
>>>>>>> thread into _thread_in_vm so that we can enter the safe point??
>>>>>>>
>>>>>>>
>>>>>>> #0 0x0000003075430265 in raise () from /lib64/libc.so.6
>>>>>>> #1 0x0000003075431d10 in abort () from /lib64/libc.so.6
>>>>>>> #2 0x00002aaaab52cfdb in os::abort (dump_core=true) at
>>>>>>> /home/iklam/jdk/hsx24/src/os/linux/vm/os_linux.cpp:1584
>>>>>>> #3 0x00002aaaab6ded47 in VMError::report_and_die
>>>>>>> (this=0x400fe6c0) at
>>>>>>> /home/iklam/jdk/hsx24/src/share/vm/utilities/vmError.cpp:1026
>>>>>>> #4 0x00002aaaab1031ae in report_vm_error (file=0x2aaaab964e40
>>>>>>> "/home/iklam/jdk/hsx24/src/share/vm/runtime/thread.cpp", line=899,
>>>>>>> error_msg=0x2aaaab7f20af "fatal error",
>>>>>>> detail_msg=0x2aaaab968404
>>>>>>> "LEAF method calling lock?")
>>>>>>> at /home/iklam/jdk/hsx24/src/share/vm/utilities/debug.cpp:227
>>>>>>> #5 0x00002aaaab10402a in report_fatal (file=0x2aaaab964e40
>>>>>>> "/home/iklam/jdk/hsx24/src/share/vm/runtime/thread.cpp", line=899,
>>>>>>> message=0x2aaaab968404 "LEAF method calling lock?") at
>>>>>>> /home/iklam/jdk/hsx24/src/share/vm/utilities/debug.cpp:232
>>>>>>> #6 0x00002aaaab689187 in Thread::check_for_valid_safepoint_state
>>>>>>> (this=0x613800, potential_vm_operation=true)
>>>>>>> at /home/iklam/jdk/hsx24/src/share/vm/runtime/thread.cpp:899
>>>>>>> #7 0x00002aaaab70ccff in VMThread::execute (op=0x400feb60) at
>>>>>>> /home/iklam/jdk/hsx24/src/share/vm/runtime/vmThread.cpp:597
>>>>>>> #8 0x00002aaaab52b6ec in os::dll_load (filename=0x745240
>>>>>>> "/scratch/iklam/jdk/hsx24/test/runtime/JTwork/scratch/libtest-rwx.so",
>>>>>>>
>>>>>>> ebuf=0x400ff070 "\320\307j", ebuflen=1024) at
>>>>>>> /home/iklam/jdk/hsx24/src/os/linux/vm/os_linux.cpp:1898
>>>>>>> #9 0x00002aaaab368a3b in JVM_LoadLibrary (name=0x745240
>>>>>>> "/scratch/iklam/jdk/hsx24/test/runtime/JTwork/scratch/libtest-rwx.so")
>>>>>>>
>>>>>>> at /home/iklam/jdk/hsx24/src/share/vm/prims/jvm.cpp:3644
>>>>>>> #10 0x00002aaaac1ad92f in
>>>>>>> Java_java_lang_ClassLoader_00024NativeLibrary_load () from
>>>>>>> /scratch/iklam/jdk/official/jdk1.7.0_09/jre/lib/amd64/libjava.so
>>>>>>> #11 0x00002aaaac649b9b in ?? ()
>>>>>>> #12 0x00000000400ff6b0 in ?? ()
>>>>>>> #13 0x00002aaaac62c298 in ?? ()
>>>>>>> #14 0x00000000400ff660 in ?? ()
>>>>>>> #15 0x0000000000000000 in ?? ()
>>>>>>>
>>>>>>>
>>>>>>>>> 2. Using safepoint to modify stack guard is still not safe,
>>>>>>>>> since not
>>>>>>>>> all Java threads stop at safepoint. Have you thought about racing
>>>>>>>>> scenarios?
>>>>>>>>
>>>>>>>> There is no guaranteed fix for this problem, we can only do
>>>>>>>> what we
>>>>>>>> can do. If a thread hits the Java guard pages while in native
>>>>>>>> then I
>>>>>>>> think that causes an abort anyway. So if the guard is disabled we
>>>>>>>> just won't abort (a good thing?) but if we hit the pthread/libc
>>>>>>>> guard
>>>>>>>> pages then we will abort anyway.
>>>>>>>>
>>>>>>>> David
>>>>>>>> -----
>>>>>>>>
>>>>>>>>> 3. typo - os_linux.cpp #1821 liner -> linker
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> -Zhengyu
>>>>>>>>>>
>>>>>>>>>> -------- Original Message --------
>>>>>>>>>> Subject: Re: RFR (S) JDK-7107135 - Stack guard pages becomes
>>>>>>>>>> writable
>>>>>>>>>> Date: Wed, 20 Feb 2013 10:40:39 -0800
>>>>>>>>>> From: Ioi Lam <ioi.lam at oracle.com>
>>>>>>>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> [Moving the discussion tohotspot-runtime-dev at openjdk.java.net]
>>>>>>>>>>
>>>>>>>>>> The original request for review is cut-and-pasted atthe end
>>>>>>>>>> of this
>>>>>>>>>> message.
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> - Ioi
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 02/20/2013 08:25 AM, Dean Long wrote:
>>>>>>>>>>> On 2/20/2013 3:01 AM, David Holmes wrote:
>>>>>>>>>>>> On 20/02/2013 6:32 PM, Dean Long wrote:
>>>>>>>>>>>>> On 2/20/2013 12:12 AM, David Holmes wrote:
>>>>>>>>>>>>>> On 20/02/2013 3:29 PM, Dean Long wrote:
>>>>>>>>>>>>>>> On 2/19/2013 6:48 PM, David Holmes wrote:
>>>>>>>>>>>>>>>> On 20/02/2013 11:39 AM, Dean Long wrote:
>>>>>>>>>>>>>>>>> On 2/19/2013 5:35 PM, David Holmes wrote:
>>>>>>>>>>>>>>>>>> On 20/02/2013 11:24 AM, Dean Long wrote:
>>>>>>>>>>>>>>>>>>> If os::Linux::default_guard_size() returns non-zero, we
>>>>>>>>>> set an
>>>>>>>>>>>>>>>>>>> attribute
>>>>>>>>>>>>>>>>>>> for pthread_create telling it to create guard pages for
>>>>>>>>>>>>>>>>>>> us.
>>>>>>>>>>>>>>>>>>> These guard pages aren't set to rwx by the dynamic
>>>>>>>>>>>>>>>>>>> linker
>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>> pthreads knows about them.
>>>>>>>>>>>>>>>>>> Are you saying that these guard pages don't get reset
>>>>>>>>>>>>>>>>>> when
>>>>>>>>>> the
>>>>>>>>>>>>>>>>>> library
>>>>>>>>>>>>>>>>>> is loaded (or that they will at least get repaired
>>>>>>>>>> afterwards) ?
>>>>>>>>>>>>>>>>> They will not be reset.
>>>>>>>>>>>>>>>> Can you point me to any docs or the source code that
>>>>>>>>>> demonstrates
>>>>>>>>>>>>>>>> this? From previous discussions the OS only allowed you to
>>>>>>>>>> set page
>>>>>>>>>>>>>>>> bits not read them, and hence not add them. So I'd like to
>>>>>>>>>> see how
>>>>>>>>>>>>>>>> they are achieving this.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The magic happens here in change_stack_perm():
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> http://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/allocatestack.c#l314
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> But that simply makes the stack readable, writable and
>>>>>>>>>> executable but
>>>>>>>>>>>>>> skipping over the guard pages. The initial protection of the
>>>>>>>>>> guard
>>>>>>>>>>>>>> pages happens here:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 634 if (mprotect (guard, guardsize,
>>>>>>>>>>>>>> PROT_NONE) != 0)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> but I don't see anything in this code that prevents the
>>>>>>>>>>>>>> guard
>>>>>>>>>> pages
>>>>>>>>>>>>>> from being modified when a library with executable stack is
>>>>>>>>>> loaded?
>>>>>>>>>>>>> The code path that does the modifying looks like: dlopen -->
>>>>>>>>>> ... -->
>>>>>>>>>>>>> __make_stacks_executable --> change_stack_perm.
>>>>>>>>>>>> Ah I see. The above is what causes the problem, but it
>>>>>>>>>>>> skips the
>>>>>>>>>>>> built-in guard pages.
>>>>>>>>>>>>
>>>>>>>>>>>> I'm sure someone thought this made sense at some point. :(
>>>>>>>>>>>> But it
>>>>>>>>>>>> sure seems like a bug to me.
>>>>>>>>>>>>
>>>>>>>>>>> Which part seems buggy, changing the permissions? It seems
>>>>>>>>>> necessary
>>>>>>>>>>> as long as executable stacks is
>>>>>>>>>>> a supported feature. It would be nice if dlopen() would just
>>>>>>>>>> fail to
>>>>>>>>>>> load these libraries based on some flag
>>>>>>>>>>> in the executable's ELF file. I think the existing flag
>>>>>>>>>>> means "no
>>>>>>>>>>> executable stacks used by this module",
>>>>>>>>>>> but "no executable stacks used by this process" would make more
>>>>>>>>>> sense
>>>>>>>>>>> for Java.
>>>>>>>>>>>
>>>>>>>>>>> dl
>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> David
>>>>>>>>>>>>
>>>>>>>>>>>>>> Also note that for user supplied stack memory there are no
>>>>>>>>>> guard pages
>>>>>>>>>>>>>> added.
>>>>>>>>>>>>>>
>>>>>>>>>>>>> OK.
>>>>>>>>>>>>>
>>>>>>>>>>>>> dl
>>>>>>>>>>>>>
>>>>>>>>>>>>>> David
>>>>>>>>>>>>>> -----
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I wish there was a way to change the guardsize after the
>>>>>>>>>> thread is
>>>>>>>>>>>>>>> created.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> dl
>>>>>>>>>>>>>>>
>>>>>>>>>> Please review:
>>>>>>>>>> http://javaweb.us.oracle.com/~iklam/webrev/7107135/stack_guard_001/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Bug: Stack guard pages are no more protected after loading a
>>>>>>>>>> shared
>>>>>>>>>> library with executable stack
>>>>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-7107135
>>>>>>>>>>
>>>>>>>>>> Background:
>>>>>>>>>>
>>>>>>>>>> Recent versions of Linux support Non-Executable Stack
>>>>>>>>>> protection -- by default, the stack is made
>>>>>>>>>> non-executable to
>>>>>>>>>> prevent code injection via overflowing on-stack buffers.
>>>>>>>>>>
>>>>>>>>>> However, some old Linux DLLs require the stack to be
>>>>>>>>>> executable. For backwards compatibility, after loading
>>>>>>>>>> such
>>>>>>>>>> DLLs, the Linux dynamic loader makes the stack executable.
>>>>>>>>>>
>>>>>>>>>> Due to a limitation of the Linux system call API, the
>>>>>>>>>> Linux
>>>>>>>>>> dynamic loader makes the stack readable/writable as
>>>>>>>>>> well. This
>>>>>>>>>> disables Java's stack guard.
>>>>>>>>>>
>>>>>>>>>> Summary of fix:
>>>>>>>>>>
>>>>>>>>>> 1. Check if DLL requires executable stack by inspecting
>>>>>>>>>> ELF
>>>>>>>>>> header.
>>>>>>>>>> 2. Enter a Safepoint and load such DLLs in the VM thread.
>>>>>>>>>> - immediately after loading, change all Java stack
>>>>>>>>>> guards
>>>>>>>>>> back to PROT_NONE.
>>>>>>>>>> 3. Leave Safepoint to resume Java execution.
>>>>>>>>>>
>>>>>>>>>> I also added a global flag LoadExecStackDllInVMThread
>>>>>>>>>> to load
>>>>>>>>>> such "bad" DLLs outside of the safepoint (in case the DLL
>>>>>>>>>> invokes JNI functions inside static constructors, which
>>>>>>>>>> are
>>>>>>>>>> executed before dlopen() returns).
>>>>>>>>>>
>>>>>>>>>> Note:
>>>>>>>>>>
>>>>>>>>>> I got this code from an outside contributor and I don't
>>>>>>>>>> really
>>>>>>>>>> understand what this block does. Please comment if it's
>>>>>>>>>> correct:
>>>>>>>>>>
>>>>>>>>>> 1877: ThreadInVMfromNative tiv(JavaThread::current());
>>>>>>>>>> 1878: debug_only(VMNativeEntryWrapper vew;)
>>>>>>>>>>
>>>>>>>>>> Tests executed:
>>>>>>>>>>
>>>>>>>>>> * JPRT
>>>>>>>>>> * UTE (vm.quick.testlist)
>>>>>>>>>> * JTREG (hotspot/tests/runtime,
>>>>>>>>>> hotspot/tests/closed/runtime)
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Ioi
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130305/e9bd7681/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list