RFR (S) JDK-7107135 - Stack guard pages becomes writable

Ioi Lam ioi.lam at oracle.com
Thu Feb 28 12:51:58 PST 2013


Hi,

Could someone give an official review for this bug so I could get it 
committed (to hsx24)?

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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>



More information about the hotspot-runtime-dev mailing list