[llvm] [BOLT] Gadget scanner: account for BRK when searching for auth oracles (PR #137975)
Anatoly Trosinenko via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 31 09:47:57 PDT 2025
https://github.com/atrosinenko updated https://github.com/llvm/llvm-project/pull/137975
>From 12dc41e6881281c03e6f434c6647939c7f5b68fa Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Wed, 30 Apr 2025 16:08:10 +0300
Subject: [PATCH 1/2] [BOLT] Gadget scanner: account for BRK when searching for
auth oracles
An authenticated pointer can be explicitly checked by the compiler via a
sequence of instructions that executes BRK on failure. It is important
to recognize such BRK instruction as checking every register (as it is
expected to immediately trigger an abnormal program termination) to
prevent false positive reports about authentication oracles:
autia x2, x3
autia x0, x1
; neither x0 nor x2 are checked at this point
eor x16, x0, x0, lsl #1
tbz x16, #62, on_success ; marks x0 as checked
; end of BB: for x2 to be checked here, it must be checked in both
; successor basic blocks
on_failure:
brk 0xc470
on_success:
; x2 is checked
ldr x1, [x2] ; marks x2 as checked
---
bolt/include/bolt/Core/MCPlusBuilder.h | 14 ++++++
bolt/lib/Passes/PAuthGadgetScanner.cpp | 13 +++++-
.../Target/AArch64/AArch64MCPlusBuilder.cpp | 24 ++++++++--
.../AArch64/gs-pauth-address-checks.s | 44 +++++++++----------
.../AArch64/gs-pauth-authentication-oracles.s | 9 ++--
.../AArch64/gs-pauth-signing-oracles.s | 6 +--
6 files changed, 75 insertions(+), 35 deletions(-)
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index f902a8c43cd1d..d79174f13f502 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -718,6 +718,20 @@ class MCPlusBuilder {
return false;
}
+ /// Returns true if Inst is a trap instruction.
+ ///
+ /// Tests if Inst is an instruction that immediately causes an abnormal
+ /// program termination, for example when a security violation is detected
+ /// by a compiler-inserted check.
+ ///
+ /// @note An implementation of this method should likely return false for
+ /// calls to library functions like abort(), as it is possible that the
+ /// execution state is partially attacker-controlled at this point.
+ virtual bool isTrap(const MCInst &Inst) const {
+ llvm_unreachable("not implemented");
+ return false;
+ }
+
virtual bool isBreakpoint(const MCInst &Inst) const {
llvm_unreachable("not implemented");
return false;
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index f928dd49edb25..65c84ebc8c4f4 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -1078,6 +1078,15 @@ class DstSafetyAnalysis {
dbgs() << ")\n";
});
+ // If this instruction terminates the program immediately, no
+ // authentication oracles are possible past this point.
+ if (BC.MIB->isTrap(Point)) {
+ LLVM_DEBUG({ traceInst(BC, "Trap instruction found", Point); });
+ DstState Next(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
+ Next.CannotEscapeUnchecked.set();
+ return Next;
+ }
+
// If this instruction is reachable by the analysis, a non-empty state will
// be propagated to it sooner or later. Until then, skip computeNext().
if (Cur.empty()) {
@@ -1185,8 +1194,8 @@ class DataflowDstSafetyAnalysis
//
// A basic block without any successors, on the other hand, can be
// pessimistically initialized to everything-is-unsafe: this will naturally
- // handle both return and tail call instructions and is harmless for
- // internal indirect branch instructions (such as computed gotos).
+ // handle return, trap and tail call instructions. At the same time, it is
+ // harmless for internal indirect branch instructions, like computed gotos.
if (BB.succ_empty())
return createUnsafeState();
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index 973261765f951..b9ce137e83530 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -382,10 +382,9 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
// the list of successors of this basic block as appropriate.
// Any of the above code sequences assume the fall-through basic block
- // is a dead-end BRK instruction (any immediate operand is accepted).
+ // is a dead-end trap instruction.
const BinaryBasicBlock *BreakBB = BB.getFallthrough();
- if (!BreakBB || BreakBB->empty() ||
- BreakBB->front().getOpcode() != AArch64::BRK)
+ if (!BreakBB || BreakBB->empty() || !isTrap(BreakBB->front()))
return std::nullopt;
// Iterate over the instructions of BB in reverse order, matching opcodes
@@ -1744,6 +1743,25 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
Inst.addOperand(MCOperand::createImm(0));
}
+ bool isTrap(const MCInst &Inst) const override {
+ if (Inst.getOpcode() != AArch64::BRK)
+ return false;
+ // Only match the immediate values that are likely to indicate this BRK
+ // instruction is emitted to terminate the program immediately and not to
+ // be handled by a SIGTRAP handler, for example.
+ switch (Inst.getOperand(0).getImm()) {
+ case 0xc470:
+ case 0xc471:
+ case 0xc472:
+ case 0xc473:
+ // Explicit Pointer Authentication check failed, see
+ // AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue().
+ return true;
+ default:
+ return false;
+ }
+ }
+
bool isStorePair(const MCInst &Inst) const {
const unsigned opcode = Inst.getOpcode();
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s b/bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s
index 3f982ddaf6e38..74f276197923f 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s
@@ -31,7 +31,7 @@ resign_xpaci_good:
xpaci x16
cmp x0, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -46,7 +46,7 @@ resign_xpacd_good:
xpacd x16
cmp x0, x16
b.eq 1f
- brk 0x1234
+ brk 0xc473
1:
pacda x0, x2
ret
@@ -117,7 +117,7 @@ resign_xpaci_unrelated_auth_and_check:
xpaci x16
cmp x0, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x10, x2
ret
@@ -139,7 +139,7 @@ resign_xpaci_wrong_pattern_1:
xpaci x16
cmp x0, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -157,7 +157,7 @@ resign_xpaci_wrong_pattern_2:
xpaci x0 // x0 instead of x16
cmp x0, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -174,7 +174,7 @@ resign_xpaci_wrong_pattern_3:
xpaci x16
cmp x16, x16 // x16 instead of x0
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -191,7 +191,7 @@ resign_xpaci_wrong_pattern_4:
xpaci x16
cmp x0, x0 // x0 instead of x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -208,7 +208,7 @@ resign_xpaci_wrong_pattern_5:
mov x16, x16 // replace xpaci with a no-op instruction
cmp x0, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -228,7 +228,7 @@ resign_xpaclri_good:
xpaclri
cmp x30, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x30, x2
@@ -246,7 +246,7 @@ xpaclri_check_keeps_lr_safe:
xpaclri // clobbers LR
cmp x30, x16
b.eq 1f
- brk 0x1234 // marks LR as trusted and safe-to-dereference
+ brk 0xc471 // marks LR as trusted and safe-to-dereference
1:
ret // not reporting non-protected return
.size xpaclri_check_keeps_lr_safe, .-xpaclri_check_keeps_lr_safe
@@ -265,7 +265,7 @@ xpaclri_check_requires_safe_lr:
xpaclri
cmp x30, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
ret
.size xpaclri_check_requires_safe_lr, .-xpaclri_check_requires_safe_lr
@@ -283,7 +283,7 @@ resign_xpaclri_wrong_reg:
xpaclri // ... but xpaclri still operates on x30
cmp x20, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x20, x2
@@ -303,7 +303,7 @@ resign_checked_not_authenticated:
xpaci x16
cmp x0, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -323,7 +323,7 @@ resign_checked_before_authenticated:
xpaci x16
cmp x0, x16
b.eq 1f
- brk 0x1234
+ brk 0xc471
1:
autib x0, x1
pacia x0, x2
@@ -339,7 +339,7 @@ resign_high_bits_tbz_good:
autib x0, x1
eor x16, x0, x0, lsl #1
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -378,7 +378,7 @@ resign_high_bits_tbz_wrong_bit:
autib x0, x1
eor x16, x0, x0, lsl #1
tbz x16, #63, 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -393,7 +393,7 @@ resign_high_bits_tbz_wrong_shift_amount:
autib x0, x1
eor x16, x0, x0, lsl #2
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -408,7 +408,7 @@ resign_high_bits_tbz_wrong_shift_type:
autib x0, x1
eor x16, x0, x0, lsr #1
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -423,7 +423,7 @@ resign_high_bits_tbz_wrong_pattern_1:
autib x0, x1
eor x16, x0, x0, lsl #1
tbz x17, #62, 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -438,7 +438,7 @@ resign_high_bits_tbz_wrong_pattern_2:
autib x0, x1
eor x16, x10, x0, lsl #1
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -453,7 +453,7 @@ resign_high_bits_tbz_wrong_pattern_3:
autib x0, x1
eor x16, x0, x10, lsl #1
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc471
1:
pacia x0, x2
ret
@@ -648,7 +648,7 @@ many_checked_regs:
xpacd x16 // ...
cmp x2, x16 // ...
b.eq 2f // end of basic block
- brk 0x1234
+ brk 0xc473
2:
pacdza x0
pacdza x1
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s b/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s
index c314bc7cfe5a3..f44ba21b9d484 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s
@@ -79,7 +79,7 @@ good_explicit_check:
autia x0, x1
eor x16, x0, x0, lsl #1
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc470
1:
ret
.size good_explicit_check, .-good_explicit_check
@@ -373,7 +373,7 @@ good_explicit_check_multi_bb:
1:
eor x16, x0, x0, lsl #1
tbz x16, #62, 2f
- brk 0x1234
+ brk 0xc470
2:
cbz x1, 3f
nop
@@ -685,8 +685,7 @@ good_address_arith_nocfg:
.globl good_explicit_check_unrelated_reg
.type good_explicit_check_unrelated_reg, at function
good_explicit_check_unrelated_reg:
-// CHECK-LABEL: GS-PAUTH: authentication oracle found in function good_explicit_check_unrelated_reg, basic block {{[^,]+}}, at address
- // FIXME: The below instruction is not an authentication oracle
+// CHECK-NOT: good_explicit_check_unrelated_reg
autia x2, x3 // One of possible execution paths after this instruction
// ends at BRK below, thus BRK used as a trap instruction
// should formally "check everything" not to introduce
@@ -694,7 +693,7 @@ good_explicit_check_unrelated_reg:
autia x0, x1
eor x16, x0, x0, lsl #1
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc470
1:
ldr x4, [x2] // Right before this instruction X2 is checked - this
// should be propagated to the basic block ending with
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s b/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s
index 3a4d383ec5bc6..4d4bb7b0fb251 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s
@@ -57,7 +57,7 @@ good_sign_auted_checked_brk:
autda x0, x2
eor x16, x0, x0, lsl #1
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc472
1:
pacda x0, x1
ret
@@ -351,7 +351,7 @@ good_sign_auted_checked_brk_multi_bb:
1:
eor x16, x0, x0, lsl #1
tbz x16, #62, 2f
- brk 0x1234
+ brk 0xc472
2:
cbz x4, 3f
nop
@@ -705,7 +705,7 @@ good_resign_with_increment_brk:
add x0, x0, #8
eor x16, x0, x0, lsl #1
tbz x16, #62, 1f
- brk 0x1234
+ brk 0xc472
1:
mov x2, x0
pacda x2, x1
>From abe2b92bd35408800033d67dc53cf47bea9029a6 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Wed, 30 Jul 2025 23:03:40 +0300
Subject: [PATCH 2/2] Clarify operands of BRK, add tests
---
.../Target/AArch64/AArch64MCPlusBuilder.cpp | 9 +
.../AArch64/trap-instructions.s | 213 ++++++++++++++++++
2 files changed, 222 insertions(+)
create mode 100644 bolt/test/binary-analysis/AArch64/trap-instructions.s
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index b9ce137e83530..72f95cea6fa1d 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -1757,7 +1757,16 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
// Explicit Pointer Authentication check failed, see
// AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue().
return true;
+ case 0x1:
+ // __builtin_trap(), as emitted by Clang.
+ return true;
+ case 0x3e8: // decimal 1000
+ // __builtin_trap(), as emitted by GCC.
+ return true;
default:
+ // Some constants may indicate intentionally recoverable break-points.
+ // This is the case at least for 0xf000, which is used by
+ // __builtin_debugtrap() supported by Clang.
return false;
}
}
diff --git a/bolt/test/binary-analysis/AArch64/trap-instructions.s b/bolt/test/binary-analysis/AArch64/trap-instructions.s
new file mode 100644
index 0000000000000..7810b2d3c3626
--- /dev/null
+++ b/bolt/test/binary-analysis/AArch64/trap-instructions.s
@@ -0,0 +1,213 @@
+// RUN: %clang %cflags -march=armv8.3-a %s -o %t.exe -Wl,--emit-relocs
+// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s
+
+// Test what instructions can be used to terminate the program abnormally
+// on security violation.
+//
+// All test cases have the same structure:
+//
+// cbz x0, 1f // [a], ensures [c] is never reported as unreachable
+// autia x2, x3
+// cbz x1, 2f // [b]
+// [instruction under test]
+// 1:
+// ret // [c]
+// 2:
+// ldr x0, [x2]
+// ret
+//
+// This is to handle three possible cases: the instruction under test may be
+// considered by BOLT as
+// * trapping (and thus no-return): after being authenticated, x2 is ether
+// checked by LDR (if [b] is taken) or the program is terminated
+// immediately without leaking x2 (if [b] falls through to the trapping
+// instruction under test). Nothing is reported.
+// * non-trapping, but no-return (such as calling abort()): x2 is leaked if [b]
+// falls through. Authentication oracle is reported.
+// * non-trapping and falling-through (i.e. a regular instruction):
+// x2 is leaked by [c]. Authentication oracle is reported.
+
+ .text
+
+ .globl brk_key_ia
+ .type brk_key_ia, at function
+brk_key_ia:
+// CHECK-NOT: brk_key_ia
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ brk 0xc470
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size brk_key_ia, .-brk_key_ia
+
+ .globl brk_key_ib
+ .type brk_key_ib, at function
+brk_key_ib:
+// CHECK-NOT: brk_key_ib
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ brk 0xc471
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size brk_key_ib, .-brk_key_ib
+
+ .globl brk_key_da
+ .type brk_key_da, at function
+brk_key_da:
+// CHECK-NOT: brk_key_da
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ brk 0xc472
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size brk_key_da, .-brk_key_da
+
+ .globl brk_key_db
+ .type brk_key_db, at function
+brk_key_db:
+// CHECK-NOT: brk_key_db
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ brk 0xc473
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size brk_key_db, .-brk_key_db
+
+// The immediate operand of BRK instruction may indicate whether the instruction
+// is intended to be a non-recoverable trap: for example, for this code
+//
+// int test_trap(void) {
+// __builtin_trap();
+// return 42;
+// }
+// int test_debugtrap(void) {
+// __builtin_debugtrap();
+// return 42;
+// }
+//
+// Clang produces the following assembly:
+//
+// test_trap:
+// brk #0x1
+// test_debugtrap:
+// brk #0xf000
+// mov w0, #42
+// ret
+//
+// In GCC, __builtin_trap() uses "brk 0x3e8" (i.e. decimal 1000) and
+// __builtin_debugtrap() is not supported.
+//
+// At the time of writing these test cases, any BRK instruction is considered
+// no-return by BOLT, thus it ends its basic block and prevents falling through
+// to the next BB.
+// FIXME: Make BOLT handle __builtin_debugtrap() properly from the CFG point
+// of view.
+
+ .globl brk_gcc_builtin_trap
+ .type brk_gcc_builtin_trap, at function
+brk_gcc_builtin_trap:
+// CHECK-NOT: brk_gcc_builtin_trap
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ brk 0x3e8 // __builtin_trap()
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size brk_gcc_builtin_trap, .-brk_gcc_builtin_trap
+
+ .globl brk_clang_builtin_trap
+ .type brk_clang_builtin_trap, at function
+brk_clang_builtin_trap:
+// CHECK-NOT: brk_clang_builtin_trap
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ brk 0x1 // __builtin_trap()
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size brk_clang_builtin_trap, .-brk_clang_builtin_trap
+
+ .globl brk_clang_builtin_debugtrap
+ .type brk_clang_builtin_debugtrap, at function
+brk_clang_builtin_debugtrap:
+// CHECK-LABEL: GS-PAUTH: authentication oracle found in function brk_clang_builtin_debugtrap, basic block {{[^,]+}}, at address
+// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x2, x3
+// CHECK-NEXT: The 0 instructions that leak the affected registers are:
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ brk 0xf000 // __builtin_debugtrap()
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size brk_clang_builtin_debugtrap, .-brk_clang_builtin_debugtrap
+
+// Conservatively assume BRK with an unknown immediate operand as not suitable
+// for terminating the program on security violation.
+ .globl brk_unknown_imm
+ .type brk_unknown_imm, at function
+brk_unknown_imm:
+// CHECK-LABEL: GS-PAUTH: authentication oracle found in function brk_unknown_imm, basic block {{[^,]+}}, at address
+// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x2, x3
+// CHECK-NEXT: The 0 instructions that leak the affected registers are:
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ brk 0x3572
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size brk_unknown_imm, .-brk_unknown_imm
+
+// Conservatively assume calling the abort() function may be an unsafe way to
+// terminate the program, as there is some amount of instructions that would
+// be executed when the program state is already tampered with.
+ .globl call_abort_fn
+ .type call_abort_fn, at function
+call_abort_fn:
+// CHECK-LABEL: GS-PAUTH: authentication oracle found in function call_abort_fn, basic block {{[^,]+}}, at address
+// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x2, x3
+// CHECK-NEXT: The 0 instructions that leak the affected registers are:
+ cbz x0, 1f
+ autia x2, x3
+ cbz x1, 2f
+ b abort // a no-return tail call to abort()
+1:
+ ret
+2:
+ ldr x0, [x2]
+ ret
+ .size call_abort_fn, .-call_abort_fn
+
+ .globl main
+ .type main, at function
+main:
+ mov x0, 0
+ ret
+ .size main, .-main
More information about the llvm-commits
mailing list