coroutine/arm64/Context.S: Support PAC/BTI #9306
Conversation
4b300b6 to
3bd5778
Compare
|
My understanding is that this is insufficient. What you are asking for, is for a shadow stack for call/return/jump instructions, but I don't think this metadata solves that problem. IIUC, this metadata change indicates that the code implements BTI/PAC without actually implementing it in the coroutine. Feel free to correct me if I'm wrong. |
|
Ah, good point. I thought that this is a leaf function and we don't need to care about the protection stuff. But on second thought, the function reads the return address from the shadow stack, so we still need to sign the pointer. Also apparently we also need BTI landing pad. Will push some followup fixes. |
|
@ioquatix Just curious: why x30 (LR) is saved twice in ( If we stick to use the return address stored in - # Load return address into x4
- ldr x4, [sp, 0xa0]
+ # Load return address into x17
+ ldr x17, [sp, 0xa0]
# Pop stack frame
add sp, sp, 0xb0
+ mov x16, sp
+
+ autia1716
- # Jump to return address (in x4)
- ret x4
+ # Jump to return address (in x17)
+ ret x17If we can just use # Load return address into x4
- ldr x4, [sp, 0xa0]
# Pop stack frame
add sp, sp, 0xb0
+ autiasp
- # Jump to return address (in x4)
- ret x4
+ # Jump to return address (in x30)
+ retAlso for the case if the target doesn't need ISA backward compatibility, we can combine |
4918ee0 to
d73ec62
Compare
|
I tested current patch, but unfortunately, the problem persists. |
|
@ggardet Which configure option did you use? I tested Side note: The configure command I proposed uses |
|
Here is my I added EDIT: |
Right, that's why I added |
|
@ggardet Also please note that I added |
Ok, that works now. It looks like some |
|
@ggardet I guess you are setting |
|
I read the documentation, thanks for the links. I was under the impression we'd need to implement a shadow stack like how it works with CET. However, it seems like this is sufficient. I'm a little cautious about merging this so soon before 3.3.0 - what are your thoughts on trying to get in before the next release? |
Probably because I was not an expert at the assembler and just did my best to get something that looked good enough. Feel free to improve it as you see fit :) |
I think it's relatively low-risk to get this in 3.3.0 release, let's merge soon after some fixup commits.
Okay, thank you for clarifying! |
d73ec62 to
b38835f
Compare
|
The RJIT workflow failure is unrelated to this change, it's also failing on master. https://github.com/ruby/ruby/actions/runs/7294360546/job/19879066887 |
We don't need to save/restore x30 twice, and we can just use `ret`, which uses x30 as return address register instead of explicit `ret <reg>` instruction. This also allows us to use `autiasp` instead of `autia1716` and we can skip setting SP/LR to x16/x17. Also the size of register save area is shrunk by 16 bytes due to the removal of extra x30 save/restore.
b38835f to
e13bfc0
Compare
|
Thanks! |
Ok, that works now. |
|
|
Fixes https://bugs.ruby-lang.org/issues/20029