RFR (S) JDK-7107135 - Stack guard pages becomes writable
Dmitry Samersoff
dmitry.samersoff at oracle.com
Thu Feb 21 04:38:58 PST 2013
David,
> Somewhere it's proposed to refuse loading libraries that require
> executable stack.
If my memory is not bogus this discussion has very long history and
I still don't see clear justification why we should load library with
executable stack instead of just abandon it (in generic case - I
understand SAP's concerns)
-Dmitry
On 2013-02-21 14:35, David Holmes wrote:
> 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
>>>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
More information about the hotspot-runtime-dev
mailing list