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

[Volker Simonis]

Hi Ioi,

could you please always post webrev and bug URLs which are visible
from non-Oracle employees as well.

I think the original bug together with a fix was submitted by a
colleague, but with the information you give it is hard to verify
this.

Regards,
Volker

On Wed, Feb 20, 2013 at 7:40 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
> [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
>