RFR (S) JDK-7107135 - Stack guard pages becomes writable
David Holmes
david.holmes at oracle.com
Thu Feb 28 22:53:00 PST 2013
Reviewed.
David
On 1/03/2013 6:51 AM, Ioi Lam wrote:
> 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