RFR (S) JDK-7107135 - Stack guard pages becomes writable
Zhengyu Gu
zhengyu.gu at oracle.com
Tue Mar 5 11:54:38 PST 2013
Look good to me.
-Zhengyu
On 3/5/2013 2:03 PM, Ioi Lam wrote:
> 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/add66399/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list