RFR (S) JDK-7107135 - Stack guard pages becomes writable
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Thu Feb 21 05:56:25 PST 2013
Hi David
> 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.
My code first checks for not-executable, and if not does the VM operation.
I.e., I do it for libs with executable stack, and for old libraries. A flag is good.
(Unfotunately I can not see the webrev Ioi proposed ...)
> 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.
Yes, I agree. Also, a loaded library could again load another library after
init is complete, so this is not controllable by the VM.
> 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.
You're right, the OS got it wrong.
Best regards,
Goetz.
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