8287325: AArch64: fix virtual threads with -XX:UseBranchProtection=pac-ret #13322
Conversation
…c-ret * Background 1. PAC-RET branch protection was initially implemented on Linux/AArch64 in JDK-8277204 [1]. 2. However, it was broken with the introduction of virtual threads [2], mainly because the continuation freeze/thaw mechanism would trigger stack copying to/from memory, whereas the saved and signed LR on the stack doesn't get re-signed accordingly. 3. PR-9067 [3] tried to implement the re-sign part, but it was not accepted because option "PreserveFramePointer" is always turned on by PAC-RET but this would slow down virtual threads by ~5-20x. 4. As a workaround, JDK-8288023 [4] disables PAC-RET when preview language features are enabled. Note that virtual thread is one preview feature then. 5. Virtual thread will become a permanent feature in JDK-21 [5][6]. * Goal This patch aims to make PAC-RET compatible with virtual threads. * Requirements of virtual threads R-1: Option "PreserveFramePointer" should be turned off. That is, PAC-RET implementation should not rely on frame pointer FP. Otherwise, the fast path in stack copying will never be taken. R-2: Use some invariant values to stack copying as the modifier, so as to avoid the PAC re-sign for continuation thaw, as the fast path in stack copying doesn't walk the frame. Note that more details can be found in the discussion [3]. * Investigation We considered to use (relative) stack pointer SP, thread ID, PACStack [7] and value zero as the candidate modifier. 1. SP: In some scenarios, we need to authenticate the return address in places where the current SP doesn't match the SP on function entry. E.g. see the usage in Runtime1::generate_handle_exception(). Hence, neither absolute nor relative SP works. 2. thread ID (tid): It's invariant to virtual thread, but it's nontrivial to access it from the JIT side. We need 1) firstly resolve the address of current thread (See [8] as an example), and 2) get the tid field in the way like java_lang_Thread::thread_id(). I suppose this would introduce big performance overhead. Then can we turn to use "rthread" register (JavaThread object address) as the modifier? Unfortunately, it's not an invariant to virtual threads and PAC re-sign is still needed. 3. PACStack uses the signed return address of caller as the modifier to sign the callee's return address. In this way, we get one PACed call chain. The modifier should be saved into somewhere around the frame record. Inevitably, FP should be preserved to make it easy to find this modifier in case of some exception scenarios (Recall the reason why we fail to use SP as the modifier). Finally, we choose to use value zero as the modifier. Trivially, it's compatible with virtual threads. However, compared to FP modifier, this solution would reduce the strength of PAC-RET protection to some extent. E.g., you get the same authentication code for each call to the function, whereas using FP gives you different codes as long as the stack depth is different. * Implementation of Zero modifier Here list the key updates of this patch. 1. vm_version_aarch64.cpp Remove the constraint on "enable-preview" and "PreserveFramePointer". 2. macroAssembler_aarch64.cpp For utility protect_return_address(), 1) use PACIAZ/PACIZA instructions directly. 2) argument "temp_reg" is removed since all functions use the same modifier. 3) all the use sites are updated accordingly. This involves the updates in many files. Similar updates are done to utility authenticate_return_address(). Besides, aarch64.ad and AArch64TestAssembler.java are updated accordingly. 3. pauth_linux_aarch64.inline.hpp For utilities pauth_sign_return_address() and pauth_authenticate_return_address(), remove the second argument and pass value zero to r16 register. Similarly, all the use sites are updated as well. This involves the updates in many files. 4. continuationHelper_aarch64.inline.hpp Introduce return_pc_at() and patch_pc_at() to avoid directly reading the saved PC or writing new signed PC on the stack in shared code. 5. Minor updates 1) sharedRuntime_aarch64.cpp: Add the missing authenticate_return_address() use for function gen_continuation_enter(). In functions generate_deopt_blob() and generate_uncommon_trap_blob(), remove the authentication on the caller (3) frame since the return address is not used. 2) stubGenerator_aarch64.cpp: Add the missing authenticate_return_address() use for function generate_cont_thaw(). 3) runtime.cpp: enable the authentication. * Test 1. Cross compilations on arm32/s390/ppc/riscv passed. 2. zero build and x86 build passed. 3. tier1~3 passed on Linux/AArch64 w/ and w/o PAC-RET. Co-Developed-by: Nick Gasson <Nick.Gasson@arm.com> [1] https://bugs.openjdk.org/browse/JDK-8277204 [2] https://openjdk.org/jeps/425 [3] openjdk#9067 [4] https://bugs.openjdk.org/browse/JDK-8288023 [5] https://bugs.openjdk.org/browse/JDK-8301819 [6] https://openjdk.org/jeps/444 [7] https://www.usenix.org/conference/usenixsecurity21/presentation/liljestrand [8] openjdk#10441
|
👋 Welcome back haosun! A progress list of the required criteria for merging this PR into |
|
/contributor add ngasson |
|
@shqking |
Webrevs
|
|
This is going to take time to review. It's very intrusive, and PAC/RET places a burden on maintainers because it will double the testing effort whenever HotSpot is changed. |
|
Using SP seems like the right way to go. Can't we compute the correct SP value to use in Runtime1::generate_handle_exception()? |
Thanks for your question, Dean. In my local test, I always use My concern is that using absolute SP is incompatible with virtual thread, since PAC re-sign is still needed due to the stack copying process (See Requirement-2 in the commit message). Alternatively, we may want to use relative SP as the modifier. However, I didn't know how to get such an initial SP value. Do you have any idea? |
|
@shqking, to get a relative SP, I think you would want to subtract SP from thread->last_continuation()->entry_sp(). |
Rename return_pc_at to return_address_at. Rename patch_pc_at to patch_return_address_at.
Thanks for your hint. I will take a try. |
Different from [1], this is another implementation of PAC-RET branch
protection on Linux/AArch64, i.e. using relative SP as the modifier.
Note that relative SP = signing SP - initial SP, where signing SP is the
lowest address of the frame record where LR is saved, and initial SP is
thread->last_continuation()->entry_sp(). For non-virtual thread, initial
SP is zero, and relative SP becomes absolute SP in this case.
```
// For virtual thread:
| |
|==========|
| | // Continuation.run
| |
|==========|\
| | | ContinuationEntry Address
| | | high
initial SP ---> |==========|/ |
| | |
| ... | \|/
| | low
|==========|\
| LR | |
| FP | | frame record
signing SP ---> |----------|/
| |
| |
```
Here lists the key difference from zero modifier [1] in implementation.
1. initial SP: We need access the thread register (i.e. rthread) to load
the initial SP. However, rthread is not assigned yet at the time of
stack frame creation for make_upcall_stub() and generate_call_stub().
2. initial SP: For shared code, we utilize
Continuation::get_continuation_entry_for_sp() to find the continuation
which the signing SP belongs to. See the update in
continuationHelper_aarch64.inline.hpp, frame_aarch64.cpp and
frame_aarch64.inline.hpp.
3. signing SP: It's easy to compute the signing SP in most cases, but
special handling is needed for patch_callers_callsite() as
push_CPU_state() moves the SP a lot.
4. relative SP: One temporary register is needed to save the result. We
choose to use rscratch1. Note that for the safety consideration, we
save/restore it on the stack before/after PAC. See the code in
protect_return_address(Register thread) and
authenticate_return_address(Register thread).
Test:
Ran tier1~3 on Linux/AArch64 w/ and w/o PAC-RET and got only one new
failure, i.e. jdk/internal/vm/Continuation/Fuzz.java
TODOs:
1. Fix the jtreg failure: jdk/internal/vm/Continuation/Fuzz.java
2. Following [1], we didn't authenticate the return address in the
following functons. We should try to enable the authentication.
ContinuationHelper::return_address_at()
ContinuationHelper::Frame::real_pc()
StackChunkFrameStream<frame_kind>::get_pc()
3. Some tests in stubRoutines.cpp are disabled, because these tests are
conducted during the VM initialization phase and thread register is not
assigned yet. We may want to move these tests to some other places.
4. We thought it's the usual convention that any MacroAssembler call can
clobber rscratch1 or rscratch2 registers. We cannot do that in
protect_return_address() and authenticate_return_address(). Here shows
an exception case [2]. We may need revisit all the usages of rscratch1
and rscratch2 to check if this convention can be guaranteed.
Co-Developed-by: Nick Gasson Nick.Gasson@arm.com
[1] openjdk#13322
[2] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp#L2936-L2949
|
Thanks @dean-long for the suggestion of using I will continue to fix the TODOs and make this solution ready for official review. relative SPDifferent from [1], this is another implementation of PAC-RET branch protection on Linux/AArch64, i.e. using relative SP as the modifier. Note that ImplementationHere lists the key difference from zero modifier [1] in implementation.
TestRan tier1~3 on Linux/AArch64 w/ and w/o PAC-RET and got only one new failure, i.e. TODOs
[1] #13322 |
Generally speaking, that is the rule. If LR is not live in the snippet you mentioned, we could use LR instead of rscratch1. |
Thanks for your comment. |
|
What I thinking was to use the absolute SP during call/return, but "relativize" it on continuation freeze and then rewrite to the new absolute address on thaw. |
Thanks for your review. |
OK, that makes sense. |
|
@shqking this pull request can not be integrated into git checkout jdk-8287325
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
openjdk
bot
added
the
merge-conflict
openjdk
bot
removed
the
merge-conflict
|
In the latest commit, I have reverted to the PAC-RET implementation using Tests:
@theRealAph Could you help take another look at it when you have spare time? |
Looking good. One more nit. |
|
@shqking This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
Thanks a lot for your insightful comments to make this patch better. @theRealAph I believe a second reviewer is needed. Here is a brief summary of our updates after your last review. Regarding to the
Hence, we finally revert to our initial implementation, i.e. using |
src/hotspot/os_cpu/linux_aarch64/pauth_linux_aarch64.inline.hpp
Outdated
Show resolved
Hide resolved
src/hotspot/os_cpu/linux_aarch64/pauth_linux_aarch64.inline.hpp
Outdated
Show resolved
Hide resolved
|
Thanks a lot for your reviews. I merged with the latest code in the mainline and reran the tests locally. All the tests passed. Let me integrate it. |
openjdk
bot
removed
ready
Background
PAC-RET branch protection was initially implemented on Linux/AArch64 in JDK-8277204 [1].
However, it was broken with the introduction of virtual threads [2], mainly because the continuation freeze/thaw mechanism would trigger stack copying to/from memory, whereas the saved and signed LR on the stack doesn't get re-signed accordingly.
PR-9067 [3] tried to implement the re-sign part, but it was not accepted because option "PreserveFramePointer" is always turned on by PAC-RET but this would slow down virtual threads by ~5-20x.
As a workaround, JDK-8288023 [4] disables PAC-RET when preview language features are enabled. Note that virtual thread is one preview feature then.
Virtual thread will become a permanent feature in JDK-21 [5][6].
Goal
This patch aims to make PAC-RET compatible with virtual threads.
Requirements of virtual threads
R-1: Option "PreserveFramePointer" should be turned off. That is, PAC-RET implementation should not rely on frame pointer FP. Otherwise, the fast path in stack copying will never be taken.
R-2: Use some invariant values to stack copying as the modifier, so as to avoid the PAC re-sign for continuation thaw, as the fast path in stack copying doesn't walk the frame.
Note that more details can be found in the discussion [3].
Investigation
We considered to use (relative) stack pointer SP, thread ID, PACStack [7] and value zero as the candidate modifier.
SP: In some scenarios, we need to authenticate the return address in places where the current SP doesn't match the SP on function entry. E.g. see the usage in Runtime1::generate_handle_exception(). Hence, neither absolute nor relative SP works.
thread ID (tid): It's invariant to virtual thread, but it's nontrivial to access it from the JIT side. We need 1) firstly resolve the address of current thread (See [8] as an example), and 2) get the tid field in the way like java_lang_Thread::thread_id(). I suppose this would introduce big performance overhead. Then can we turn to use "rthread" register (JavaThread object address) as the modifier? Unfortunately, it's not an invariant to virtual threads and PAC re-sign is still needed.
PACStack uses the signed return address of caller as the modifier to sign the callee's return address. In this way, we get one PACed call chain. The modifier should be saved into somewhere around the frame record. Inevitably, FP should be preserved to make it easy to find this modifier in case of some exception scenarios (Recall the reason why we fail to use SP as the modifier).
Finally, we choose to use value zero as the modifier. Trivially, it's compatible with virtual threads. However, compared to FP modifier, this solution would reduce the strength of PAC-RET protection to some extent. E.g., you get the same authentication code for each call to the function, whereas using FP gives you different codes as long as the stack depth is different.
Implementation of Zero modifier
Here list the key updates of this patch.
Remove the constraint on "enable-preview" and "PreserveFramePointer".
For utility protect_return_address(), 1) use PACIAZ/PACIZA instructions directly. 2) argument "temp_reg" is removed since all functions use the same modifier. 3) all the use sites are updated accordingly. This involves the updates in many files.
Similar updates are done to utility authenticate_return_address().
Besides, aarch64.ad and AArch64TestAssembler.java are updated accordingly.
For utilities pauth_sign_return_address() and
pauth_authenticate_return_address(), remove the second argument and pass value zero to r16 register.
Similarly, all the use sites are updated as well. This involves the updates in many files.
Introduce return_pc_at() and patch_pc_at() to avoid directly reading the saved PC or writing new signed PC on the stack in shared code.
(1) sharedRuntime_aarch64.cpp: Add the missing
authenticate_return_address() use for function gen_continuation_enter(). In functions generate_deopt_blob() and generate_uncommon_trap_blob(), remove the authentication on the caller (3) frame since the return address is not used.
(2) stubGenerator_aarch64.cpp: Add the missing
authenticate_return_address() use for function generate_cont_thaw().
(3) runtime.cpp: enable the authentication.
Test
Co-Developed-by: Nick Gasson Nick.Gasson@arm.com
[1] https://bugs.openjdk.org/browse/JDK-8277204
[2] https://openjdk.org/jeps/425
[3] #9067
[4] https://bugs.openjdk.org/browse/JDK-8288023
[5] https://bugs.openjdk.org/browse/JDK-8301819
[6] https://openjdk.org/jeps/444
[7] https://www.usenix.org/conference/usenixsecurity21/presentation/liljestrand
[8] #10441
Progress
Issue
Reviewers
Contributors
<ngasson@openjdk.org>Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13322/head:pull/13322$ git checkout pull/13322Update a local copy of the PR:
$ git checkout pull/13322$ git pull https://git.openjdk.org/jdk.git pull/13322/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 13322View PR using the GUI difftool:
$ git pr show -t 13322Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13322.diff
Webrev
Link to Webrev Comment