RFR (S) JDK-7107135 - Stack guard pages becomes writable
David Holmes
david.holmes at oracle.com
Thu Feb 21 02:35:51 PST 2013
Hi Goetz,
IIRC your original proposal was to always load libraries in the VM. We
decided this had some compatibility risk as we don't know what the
library init function might try to do, and whether that would work from
the VMThread rather than a Java thread. So after vigorous discussions on
how we might avoid the problem altogether - which we couldn't - I
proposed that we only load the libraries marked as executable-stack in
the VMThread, and that we also provide a flag to disable that if it
caused a problem with the init function. Obviously we can't completely
solve all the issues in that case.
With regard to "only libraries coming with the VM should be loaded "
that is only true when the VM is launched by a standard launcher. If the
VM is loaded into another process then that process could be loading
additional libraries concurrently with the VM being loaded. It isn't
even true with the standard launchers as an agent library could cause
loading of additional external libraries.
I find this an insidious problem, and it bugs me that the VM has to try
and clean up the mess introduced by the OS. Ideally mprotect would allow
individual bit control. Failing that it would be nice if there were a
pthread API to either register additional stack guard pages, or expand
the number provided for by the library - and in both cases they would
not be made executable by dl_open.
Cheers,
David
On 21/02/2013 8:08 PM, Lindenmaier, Goetz wrote:
> 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