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

[Ioi Lam]

[Moving the discussion to hotspot-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