On Mon, 24 Jan 2022 15:56:06 GMT, Alan Hayward <d...@openjdk.java.net> wrote:
>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>>
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>>
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>>
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>>
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>>
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>>
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix popframe failures
A few stylistic nits below. And one query.
Thanks,
David
src/hotspot/cpu/aarch64/globals_aarch64.hpp line 123:
> 121: range(1, 99) \
> 122: product(ccstr, UseBranchProtection, "none", \
> 123: "Branch Protection to use: none,standard,pac-ret") \
Nit: spaces after commas please.
src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5286:
> 5284:
> 5285: void MacroAssembler::enter()
> 5286: {
Style nit: opening braces go on the same line as the function declaration.
src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 417:
> 415: // Enable PAC if this code has been built with branch-protection and
> the CPU/OS supports it.
> 416: #ifdef __ARM_FEATURE_PAC_DEFAULT
> 417: if (_features & CPU_PACA) {
Style nit: no implicit booleans - expand as "if ( A & B != 0)"
src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 429:
> 427: #else
> 428: warning("UseROPProtection specified, but not supported in the VM.");
> 429: #endif
If we issue these warnings should `_rop_protection` still be set true?
src/hotspot/share/gc/shared/barrierSetNMethod.cpp line 58:
> 56:
> 57: address return_address = *return_address_ptr;
> 58: AARCH64_ONLY(return_address=pauth_strip_pointer(return_address));
Style nit: spaces around binary operators please. There are a couple of
occurrences.
src/hotspot/share/runtime/frame.cpp line 1115:
> 1113: AARCH64_ONLY(if (!pauth_ptr_is_raw(x)) {
> 1114: return false;
> 1115: })
Style nit: Use ifdef for multi-line code blocks.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6334
