Skip to content

[BOLT] Gadget scanner: account for BRK when searching for auth oracles #137975

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.

Sign up for GitHub

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 2 commits into from
Aug 25, 2025
Merged

[BOLT] Gadget scanner: account for BRK when searching for auth oracles #137975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6be212c
[BOLT] Gadget scanner: account for BRK when searching for auth oracles
atrosinenko Apr 30, 2025
b5f8cdb
Clarify operands of BRK, add tests
atrosinenko Jul 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions bolt/include/bolt/Core/MCPlusBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 11 additions & 2 deletions bolt/lib/Passes/PAuthGadgetScanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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();

Expand Down
33 changes: 30 additions & 3 deletions bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1744,6 +1743,34 @@ 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().
Comment on lines +1749 to +1758
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if it's a good idea to only consider pauthabi-specific BRK values in a "generic" AArch64-interface to test whether something is a trap. This "isTrap" function might get used by other analyses too...
I wonder if there would be a way to change the interface of isTrap to make it appropriately generic so that it could be used without confusion by other analyses too?

An example is this commit that makes the pac-ret analysis more accurate, which I guess hasn't been upstreamed yet: 5b3ed52

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the original patch, I tried to make isTrap function reasonably usable by non-PAuth analysis by describing it as suitable for terminating the program immediately on a security violation (compared to a generic MCPlusBuilder::isBreakpoint which does not explicitly mention any security-related properties). My original reasoning for only matching well-known constants was that somebody may use brk instruction for "recoverable" break-points by installing a SIGTRAP handler, for example.

As mentioned in 5b3ed52, there was brk 0x3e8 instruction spotted in-the-wild - turned out, it is the instruction emitted by GCC for __builtin_trap(). By the way, Clang uses brk 0x1 instead. Furthermore, as discussed in GCC bug tracker, there is __builtin_debugtrap() in Clang (but not in GCC) which is not no-return (and there is even std::breakpoint proposed for C++).

Frankly speaking, I did not succeed in observing any difference between executing __builtin_trap() and __builtin_debugtrap() on AArch64 Linux - both result in SIGTRAP. But on x86_64, these are SIGILL vs. SIGTRAP and the codegen differs on both targets: for this source code

int test_trap(void) {
  __builtin_trap();
  return 42;
}
int test_debugtrap(void) {
  __builtin_debugtrap();
  return 42;
}

Clang generates AArch64 assembly along the lines

test_trap:
        brk     #0x1

test_debugtrap:
        brk     #0xf000
        mov     w0, #42
        ret

In abe2b92, I added a dedicated test file on detecting various instructions as immediately terminating the program vs. recoverable break-points. Turned out, any BRK instruction is treated as noreturn by BOLT, thus __builtin_debugtrap() is probably understood differenlty by BOLT disassembler and by LLVM backend on AArch64 - I added a FIXME on this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for investigating this in detail!
It is unfortunate that we have to hard-code assumptions of what the meanings are of specific immediates in the brk instruction. Really, this is some form of non-documented "ABI" now.
Anyway, I agree that what you've implemented in the current version of this patch seems to be the most reasonable way to implement this.

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;
}
}

bool isStorePair(const MCInst &Inst) const {
const unsigned opcode = Inst.getOpcode();

Expand Down
44 changes: 22 additions & 22 deletions bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ resign_xpaci_good:
xpaci x16
cmp x0, x16
b.eq 1f
brk 0x1234
brk 0xc471
1:
pacia x0, x2
ret
Expand All @@ -46,7 +46,7 @@ resign_xpacd_good:
xpacd x16
cmp x0, x16
b.eq 1f
brk 0x1234
brk 0xc473
1:
pacda x0, x2
ret
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -228,7 +228,7 @@ resign_xpaclri_good:
xpaclri
cmp x30, x16
b.eq 1f
brk 0x1234
brk 0xc471
1:
pacia x30, x2

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -685,16 +685,15 @@ good_address_arith_nocfg:
.globl good_explicit_check_unrelated_reg
.type good_explicit_check_unrelated_reg,@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
// false-positive here.
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
Expand Down
6 changes: 3 additions & 3 deletions bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading