RFR (S) JDK-7107135 - Stack guard pages becomes writable
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Thu Feb 21 02:08:48 PST 2013
Hi Ioi,
the bug and fix was originally filed by me.
http://www.sapjvm.com/gl/webrevs/noexecstack/
I appreciate a lot you picked this up! We are running the code
productive in our VM since I fixed it back in 2011.
I read the mail thread as far as it is in the public. Unfortunately I can
not see your webrev, so my comments are based on my proposal
(see above).
Somewhere it's proposed to refuse loading libraries that require
executable stack.
Obviously, this would be easy to implement based on the elf file
parsing in this change.
But as was also stated in this thread, this could break existing systems.
Therefore we (sap) must deliver a VM that can deal with the old libraries.
A flag would be good.
David states that Java threads might be started before init_completed is
set.
If I understand correctly, up to this point only libraries coming with the
VM should be loaded (if at all). As these should have proper stack settings
in first place this should be fine.
You also need to add a VM entry in nativeLookup.cpp, as with criticalNatives
Libraries are loaded here:
int args_size = 1 // JNIEnv
+ (method->is_static() ? 1 : 0) // class for static methods
+ method->size_of_parameters(); // actual parameters
+ // Thread state transition for lookup of "critical natives".
+ ThreadToNativeFromVMWrapper tr( Thread::current( ) );
+
// 1) Try JNI short style
entry = lookup_critical_style(method, critical_name, "", args_size, true);
if (entry != NULL) return entry;
Best regards,
Goetz.
-----Original Message-----
From: hotspot-runtime-dev-bounces at openjdk.java.net [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of Volker Simonis
Sent: Donnerstag, 21. Februar 2013 10:04
To: Ioi Lam
Cc: hotspot-runtime-dev at openjdk.java.net
Subject: Re: RFR (S) JDK-7107135 - Stack guard pages becomes writable
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
>
More information about the hotspot-runtime-dev
mailing list