On Fri, 19 Apr 2024 07:44:17 GMT, Richard Reingruber <rr...@openjdk.org> wrote:

>> An alternative for preemptively switching the W^X thread mode on macOS with 
>> an AArch64 CPU. This implementation triggers the switch in response to the 
>> SIGBUS signal if the *si_addr* belongs to the CodeCache area. With this 
>> approach, it is now feasible to eliminate all WX guards and avoid 
>> potentially costly operations. However, no significant improvement or 
>> degradation in performance has been observed.  Additionally, considering the 
>> issue with AsyncGetCallTrace, the patched JVM has been successfully operated 
>> with [asgct_bottom](https://github.com/parttimenerd/asgct_bottom) and 
>> [async-profiler](https://github.com/async-profiler/async-profiler). 
>> 
>> Additional testing:
>> - [x] MacOS AArch64 server fastdebug *gtets*
>> - [ ] MacOS AArch64 server fastdebug *jtreg:hotspot:tier4*
>> - [ ] Benchmarking
>> 
>> @apangin and @parttimenerd could you please check the patch on your 
>> scenarios??
>
> What about granting `WXWrite` only if the current thread is in 
> `_thread_in_vm`?
> That would be more restrictive and roughly equivalent how it currently works. 
> Likely there are some places then that should be granted `WXWrite` eagerly 
> because they need `WXWrite` without `_thread_in_vm`. E.g. the JIT compiler 
> threads should have `WXWrite` and never `WXExec` (I assume) which should be 
> checked in the signal handler.

> The patch doesn't protect against native agents, as this is obviously 
> impossible. The current code doesn't do that either. For the bytecode, it 
> doesn't prevent the attacker from abusing unsafe api to modify code cache. 
> However unsafe functions are already considered "safe" and we proactively 
> enable WXWrite as well as move thread to `_thread_in_vm` state (@reinrich). 
> JITed code can't write to the cache either with or without the patch.
> 
> I totally get the sense of loss of security. But is this really the case?

I think it is. W^X is intended (amongst other things) to protect against the 
use of gadgets, from buffer overflow exploits in non-java code to ROP 
programming. At present, in order to generate code and execute it, you first 
have to be able to make the JIT code writable, then write the code, then make 
it executable. then jump to the code. And the exploit writer might have to do 
some or all of this by finding gadgets. If we were to merge this patch then all 
the attacker would have to do is write code to memory and find a way to jump to 
it, and the automatic switch-on-segfault in this patch would do the all the 
work the attacker needs.

It makes far more sense to tag those places that actually need to change W^X 
access, and only switch there.

You could argue that any switching of W^X on a write to code space, then 
switching it back on jumping (or returning) to Java code, even what we already 
do, is effectively the same thing. Kinda, but it's not on just any attempt to 
write to code space or any attempt to jump into code, it's at the places we 
choose, and we can be careful to limit those places.

But surely the JDK is not the most vulnerable part of the stack anyway? I'd 
agree with that, of course, but I don't think that's sufficient reason to 
decide to bypass an OS security mechanism.

We are trying to reduce the size of the attack surface.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/18762#issuecomment-2072639243

Reply via email to