Skip to content

[AArch64] PAUTH_PROLOGUE should not be duplicated with PAuthLR #124775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 29, 2025

Conversation

ostannard
Copy link
Collaborator

When using PAuthLR, the PAUTH_PROLOGUE expands into a sequence of instructions which takes the address of one of those instructions, and uses that address to compute the return address signature. If this is duplicated, there will be two different addresses used in calculating the signature, so the epilogue will only be correct for (at most) one of them.

This change also restricts code generation when using v8.3-A return address signing, without PAuthLR. This isn't strictly needed, as duplicating the prologue there would be valid. We could fix this by having two copies of PAUTH_PROLOGUE, with and without isNotDuplicable, but I don't think it's worth adding the extra complexity to a security feature for that.

When using PAuthLR, the PAUTH_PROLOGUE expands into a sequence of
instructions which takes the address of one of those instructions, and
uses that address to compute the return address signature. If this is
duplicated, there will be two different addresses used in calculating
the signature, so the epilogue will only be correct for (at most) one of
them.

This change also restricts code generation when using v8.3-A return
address signing, without PAuthLR. This isn't strictly needed, as
duplicating the prologue there would be valid. We could fix this by
having two copies of PAUTH_PROLOGUE, with and without isNotDuplicable,
but I don't think it's worth adding the extra complexity to a security
feature for that.
@llvmbot
Copy link
Member

llvmbot commented Jan 28, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Oliver Stannard (ostannard)

Changes

When using PAuthLR, the PAUTH_PROLOGUE expands into a sequence of instructions which takes the address of one of those instructions, and uses that address to compute the return address signature. If this is duplicated, there will be two different addresses used in calculating the signature, so the epilogue will only be correct for (at most) one of them.

This change also restricts code generation when using v8.3-A return address signing, without PAuthLR. This isn't strictly needed, as duplicating the prologue there would be valid. We could fix this by having two copies of PAUTH_PROLOGUE, with and without isNotDuplicable, but I don't think it's worth adding the extra complexity to a security feature for that.


Full diff: https://github.com/llvm/llvm-project/pull/124775.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.td (+6-1)
  • (added) llvm/test/CodeGen/AArch64/pauthlr-prologue-duplication.mir (+88)
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index d112d4f10e47d9..b77246200db645 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -1773,7 +1773,12 @@ def : InstAlias<"xpaclri", (XPACLRI), 0>;
 
 let Uses = [LR, SP], Defs = [LR] in {
 // Insertion point of LR signing code.
-def PAUTH_PROLOGUE : Pseudo<(outs), (ins), []>, Sched<[]>;
+def PAUTH_PROLOGUE : Pseudo<(outs), (ins), []>, Sched<[]> {
+  // When using PAuthLR, the address of one of the instructions this expands
+  // into is used as an input to the signature calculation, so this must not be
+  // duplicated.
+  let isNotDuplicable = 1;
+}
 // Insertion point of LR authentication code.
 // The RET terminator of the containing machine basic block may be replaced
 // with a combined RETA(A|B) instruction when rewriting this Pseudo.
diff --git a/llvm/test/CodeGen/AArch64/pauthlr-prologue-duplication.mir b/llvm/test/CodeGen/AArch64/pauthlr-prologue-duplication.mir
new file mode 100644
index 00000000000000..5e57604263793c
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/pauthlr-prologue-duplication.mir
@@ -0,0 +1,88 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple aarch64-none-elf -run-pass=block-placement -O3 -o - %s | FileCheck %s
+
+## Check that block-placement does not perform tail duplication on the
+## PAUTH_EPILOGUE instruction. If that happened, the two prologues would use
+## different addresses while calculating the return address signature, so the
+## epilogue could only be correct for (at most) one of them.
+
+--- |
+  define void @test() "frame-pointer"="non-leaf" {
+  entry:
+    ret void
+  }
+
+  declare void @f()
+...
+---
+name:            test
+body:             |
+  ; CHECK-LABEL: name: test
+  ; CHECK: bb.0.entry:
+  ; CHECK-NEXT:   successors: %bb.1(0x30000000), %bb.2(0x50000000)
+  ; CHECK-NEXT:   liveins: $w0, $w1, $lr
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   CBZW renamable $w0, %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   successors: %bb.3(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0, $w1, $lr
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   B %bb.3
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.3(0x80000000)
+  ; CHECK-NEXT:   liveins: $w1, $lr
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   renamable $w8 = MOVZWi 1, 0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3:
+  ; CHECK-NEXT:   successors: %bb.5(0x30000000), %bb.4(0x50000000)
+  ; CHECK-NEXT:   liveins: $w1, $w8, $lr
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   frame-setup PAUTH_PROLOGUE implicit-def $lr, implicit killed $lr, implicit $sp
+  ; CHECK-NEXT:   CBZW killed renamable $w1, %bb.5
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.4:
+  ; CHECK-NEXT:   successors: %bb.5(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   BL @f, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.5:
+  ; CHECK-NEXT:   BL @f, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+  ; CHECK-NEXT:   frame-destroy PAUTH_EPILOGUE implicit-def $lr, implicit killed $lr, implicit $sp
+  ; CHECK-NEXT:   TCRETURNdi @f, 0, csr_aarch64_aapcs, implicit $sp
+  bb.0.entry:
+    successors: %bb.1(0x30000000), %bb.2(0x50000000)
+    liveins: $w0, $w1, $lr
+
+    CBNZW renamable $w0, %bb.2
+
+  bb.1:
+    successors: %bb.3(0x80000000)
+    liveins: $w1, $lr
+
+    renamable $w8 = MOVZWi 1, 0
+    B %bb.3
+
+  bb.2:
+    successors: %bb.3(0x80000000)
+    liveins: $w0, $w1, $lr
+
+  bb.3:
+    successors: %bb.5(0x30000000), %bb.4(0x50000000)
+    liveins: $w1, $w8, $lr
+
+    frame-setup PAUTH_PROLOGUE implicit-def $lr, implicit killed $lr, implicit $sp
+    CBZW killed renamable $w1, %bb.5
+
+  bb.4:
+    successors: %bb.5(0x80000000)
+
+    BL @f, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+
+  bb.5:
+    BL @f, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+    frame-destroy PAUTH_EPILOGUE implicit-def $lr, implicit killed $lr, implicit $sp
+    TCRETURNdi @f, 0, csr_aarch64_aapcs, implicit $sp
+...

Copy link
Member

@vhscampos vhscampos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ostannard ostannard merged commit 36b3c43 into llvm:main Jan 29, 2025
8 checks passed
@ostannard ostannard added this to the LLVM 20.X Release milestone Jan 31, 2025
@ostannard
Copy link
Collaborator Author

/cherry-pick 36b3c43

@llvmbot
Copy link
Member

llvmbot commented Jan 31, 2025

/pull-request #125230

swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 1, 2025
…124775)

When using PAuthLR, the PAUTH_PROLOGUE expands into a sequence of
instructions which takes the address of one of those instructions, and
uses that address to compute the return address signature. If this is
duplicated, there will be two different addresses used in calculating
the signature, so the epilogue will only be correct for (at most) one of
them.

This change also restricts code generation when using v8.3-A return
address signing, without PAuthLR. This isn't strictly needed, as
duplicating the prologue there would be valid. We could fix this by
having two copies of PAUTH_PROLOGUE, with and without isNotDuplicable,
but I don't think it's worth adding the extra complexity to a security
feature for that.

(cherry picked from commit 36b3c43)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants