Skip to content

Commit b096c6b

Browse files
committed
[BOLT] Gadget scanner: optionally assume auth traps on failure
On AArch64 it is possible for an auth instruction to either return an invalid address value on failure (without FEAT_FPAC) or generate an error (with FEAT_FPAC). It thus may be possible to never emit explicit pointer checks, if the target CPU is known to support FEAT_FPAC. This commit implements an --auth-traps-on-failure command line option, which essentially makes "safe-to-dereference" and "trusted" register properties identical and disables scanning for authentication oracles completely.
1 parent b0eeddb commit b096c6b

8 files changed

+318
-227
lines changed

bolt/lib/Passes/PAuthGadgetScanner.cpp

Lines changed: 74 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "bolt/Passes/PAuthGadgetScanner.h"
1515
#include "bolt/Core/ParallelUtilities.h"
1616
#include "bolt/Passes/DataflowAnalysis.h"
17+
#include "bolt/Utils/CommandLineOpts.h"
1718
#include "llvm/ADT/STLExtras.h"
1819
#include "llvm/ADT/SmallSet.h"
1920
#include "llvm/MC/MCInst.h"
@@ -26,6 +27,11 @@ namespace llvm {
2627
namespace bolt {
2728
namespace PAuthGadgetScanner {
2829

30+
static cl::opt<bool> AuthTrapsOnFailure(
31+
"auth-traps-on-failure",
32+
cl::desc("Assume authentication instructions always trap on failure"),
33+
cl::cat(opts::BinaryAnalysisCategory));
34+
2935
[[maybe_unused]] static void traceInst(const BinaryContext &BC, StringRef Label,
3036
const MCInst &MI) {
3137
dbgs() << " " << Label << ": ";
@@ -364,6 +370,34 @@ class SrcSafetyAnalysis {
364370
return Clobbered;
365371
}
366372

373+
std::optional<MCPhysReg> getRegMadeTrustedByChecking(const MCInst &Inst,
374+
SrcState Cur) const {
375+
// This functions cannot return multiple registers. This is never the case
376+
// on AArch64.
377+
std::optional<MCPhysReg> RegCheckedByInst =
378+
BC.MIB->getAuthCheckedReg(Inst, /*MayOverwrite=*/false);
379+
if (RegCheckedByInst && Cur.SafeToDerefRegs[*RegCheckedByInst])
380+
return *RegCheckedByInst;
381+
382+
auto It = CheckerSequenceInfo.find(&Inst);
383+
if (It == CheckerSequenceInfo.end())
384+
return std::nullopt;
385+
386+
MCPhysReg RegCheckedBySequence = It->second.first;
387+
const MCInst *FirstCheckerInst = It->second.second;
388+
389+
// FirstCheckerInst should belong to the same basic block (see the
390+
// assertion in DataflowSrcSafetyAnalysis::run()), meaning it was
391+
// deterministically processed a few steps before this instruction.
392+
const SrcState &StateBeforeChecker = getStateBefore(*FirstCheckerInst);
393+
394+
// The sequence checks the register, but it should be authenticated before.
395+
if (!StateBeforeChecker.SafeToDerefRegs[RegCheckedBySequence])
396+
return std::nullopt;
397+
398+
return RegCheckedBySequence;
399+
}
400+
367401
// Returns all registers that can be treated as if they are written by an
368402
// authentication instruction.
369403
SmallVector<MCPhysReg> getRegsMadeSafeToDeref(const MCInst &Point,
@@ -386,18 +420,38 @@ class SrcSafetyAnalysis {
386420
Regs.push_back(DstAndSrc->first);
387421
}
388422

423+
// Make sure explicit checker sequence keeps register safe-to-dereference
424+
// when the register would be clobbered according to the regular rules:
425+
//
426+
// ; LR is safe to dereference here
427+
// mov x16, x30 ; start of the sequence, LR is s-t-d right before
428+
// xpaclri ; clobbers LR, LR is not safe anymore
429+
// cmp x30, x16
430+
// b.eq 1f ; end of the sequence: LR is marked as trusted
431+
// brk 0x1234
432+
// 1:
433+
// ; at this point LR would be marked as trusted,
434+
// ; but not safe-to-dereference
435+
//
436+
// or even just
437+
//
438+
// ; X1 is safe to dereference here
439+
// ldr x0, [x1, #8]!
440+
// ; X1 is trusted here, but it was clobbered due to address write-back
441+
if (auto CheckedReg = getRegMadeTrustedByChecking(Point, Cur))
442+
Regs.push_back(*CheckedReg);
443+
389444
return Regs;
390445
}
391446

392447
// Returns all registers made trusted by this instruction.
393448
SmallVector<MCPhysReg> getRegsMadeTrusted(const MCInst &Point,
394449
const SrcState &Cur) const {
450+
assert(!AuthTrapsOnFailure && "Use getRegsMadeSafeToDeref instead");
395451
SmallVector<MCPhysReg> Regs;
396452

397453
// An authenticated pointer can be checked, or
398-
std::optional<MCPhysReg> CheckedReg =
399-
BC.MIB->getAuthCheckedReg(Point, /*MayOverwrite=*/false);
400-
if (CheckedReg && Cur.SafeToDerefRegs[*CheckedReg])
454+
if (auto CheckedReg = getRegMadeTrustedByChecking(Point, Cur))
401455
Regs.push_back(*CheckedReg);
402456

403457
// ... a pointer can be authenticated by an instruction that always checks
@@ -408,19 +462,6 @@ class SrcSafetyAnalysis {
408462
if (AutReg && IsChecked)
409463
Regs.push_back(*AutReg);
410464

411-
if (CheckerSequenceInfo.contains(&Point)) {
412-
MCPhysReg CheckedReg;
413-
const MCInst *FirstCheckerInst;
414-
std::tie(CheckedReg, FirstCheckerInst) = CheckerSequenceInfo.at(&Point);
415-
416-
// FirstCheckerInst should belong to the same basic block (see the
417-
// assertion in DataflowSrcSafetyAnalysis::run()), meaning it was
418-
// deterministically processed a few steps before this instruction.
419-
const SrcState &StateBeforeChecker = getStateBefore(*FirstCheckerInst);
420-
if (StateBeforeChecker.SafeToDerefRegs[CheckedReg])
421-
Regs.push_back(CheckedReg);
422-
}
423-
424465
// ... a safe address can be materialized, or
425466
if (auto NewAddrReg = BC.MIB->getMaterializedAddressRegForPtrAuth(Point))
426467
Regs.push_back(*NewAddrReg);
@@ -463,28 +504,11 @@ class SrcSafetyAnalysis {
463504
BitVector Clobbered = getClobberedRegs(Point);
464505
SmallVector<MCPhysReg> NewSafeToDerefRegs =
465506
getRegsMadeSafeToDeref(Point, Cur);
466-
SmallVector<MCPhysReg> NewTrustedRegs = getRegsMadeTrusted(Point, Cur);
467-
468-
// Ideally, being trusted is a strictly stronger property than being
469-
// safe-to-dereference. To simplify the computation of Next state, enforce
470-
// this for NewSafeToDerefRegs and NewTrustedRegs. Additionally, this
471-
// fixes the properly for "cumulative" register states in tricky cases
472-
// like the following:
473-
//
474-
// ; LR is safe to dereference here
475-
// mov x16, x30 ; start of the sequence, LR is s-t-d right before
476-
// xpaclri ; clobbers LR, LR is not safe anymore
477-
// cmp x30, x16
478-
// b.eq 1f ; end of the sequence: LR is marked as trusted
479-
// brk 0x1234
480-
// 1:
481-
// ; at this point LR would be marked as trusted,
482-
// ; but not safe-to-dereference
483-
//
484-
for (auto TrustedReg : NewTrustedRegs) {
485-
if (!is_contained(NewSafeToDerefRegs, TrustedReg))
486-
NewSafeToDerefRegs.push_back(TrustedReg);
487-
}
507+
// If authentication instructions trap on failure, safe-to-dereference
508+
// registers are always trusted.
509+
SmallVector<MCPhysReg> NewTrustedRegs =
510+
AuthTrapsOnFailure ? NewSafeToDerefRegs
511+
: getRegsMadeTrusted(Point, Cur);
488512

489513
// Then, compute the state after this instruction is executed.
490514
SrcState Next = Cur;
@@ -521,6 +545,11 @@ class SrcSafetyAnalysis {
521545
dbgs() << ")\n";
522546
});
523547

548+
// Being trusted is a strictly stronger property than being
549+
// safe-to-dereference.
550+
assert(!Next.TrustedRegs.test(Next.SafeToDerefRegs) &&
551+
"SafeToDerefRegs should contain all TrustedRegs");
552+
524553
return Next;
525554
}
526555

@@ -1124,6 +1153,11 @@ class DataflowDstSafetyAnalysis
11241153
}
11251154

11261155
void run() override {
1156+
// As long as DstSafetyAnalysis is only computed to detect authentication
1157+
// oracles, it is a waste of time to compute it when authentication
1158+
// instructions are known to always trap on failure.
1159+
assert(!AuthTrapsOnFailure &&
1160+
"DstSafetyAnalysis is useless with faulting auth");
11271161
for (BinaryBasicBlock &BB : Func) {
11281162
if (auto CheckerInfo = BC.MIB->getAuthCheckedReg(BB)) {
11291163
LLVM_DEBUG({
@@ -1564,6 +1598,8 @@ void FunctionAnalysisContext::findUnsafeDefs(
15641598
SmallVector<PartialReport<MCPhysReg>> &Reports) {
15651599
if (PacRetGadgetsOnly)
15661600
return;
1601+
if (AuthTrapsOnFailure)
1602+
return;
15671603

15681604
auto Analysis = DstSafetyAnalysis::create(BF, AllocatorId, {});
15691605
LLVM_DEBUG({ dbgs() << "Running dst register safety analysis...\n"; });

bolt/test/binary-analysis/AArch64/cmdline-args.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ HELP-NEXT: OPTIONS:
3232
HELP-EMPTY:
3333
HELP-NEXT: BinaryAnalysis options:
3434
HELP-EMPTY:
35+
HELP-NEXT: --auth-traps-on-failure - Assume authentication instructions always trap on failure
3536
HELP-NEXT: --scanners=<value> - which gadget scanners to run
3637
HELP-NEXT: =pacret - pac-ret: return address protection (subset of "pauth")
3738
HELP-NEXT: =pauth - All Pointer Authentication scanners

bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
// RUN: %clang %cflags -march=armv8.3-a %s -o %t.exe
2-
// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s
3-
// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s
2+
// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s
3+
// RUN: llvm-bolt-binary-analysis --scanners=pauth --auth-traps-on-failure %t.exe 2>&1 | FileCheck -check-prefix=FPAC %s
4+
// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s
45

56
// The detection of compiler-generated explicit pointer checks is tested in
67
// gs-pauth-address-checks.s, for that reason only test here "dummy-load" and
78
// "high-bits-notbi" checkers, as the shortest examples of checkers that are
89
// detected per-instruction and per-BB.
910

1011
// PACRET-NOT: authentication oracle found in function
12+
// FPAC-NOT: authentication oracle found in function
1113

1214
.text
1315

bolt/test/binary-analysis/AArch64/gs-pauth-calls.s

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// RUN: %clang %cflags -march=armv8.3-a %s -o %t.exe
2-
// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s
3-
// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s
2+
// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s
3+
// RUN: llvm-bolt-binary-analysis --scanners=pauth --auth-traps-on-failure %t.exe 2>&1 | FileCheck %s
4+
// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s
45

56
// PACRET-NOT: non-protected call found in function
67

0 commit comments

Comments
 (0)