RFR (S) JDK-7107135 - Stack guard pages becomes writable
David Holmes
david.holmes at oracle.com
Thu Feb 21 19:07:33 PST 2013
<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 ??
> 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