Skip to content

[BOLT] Gadget scanner: optionally assume auth traps on failure #139778

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

Open
wants to merge 1 commit into
base: users/atrosinenko/bolt-gs-safe-jump-tables
Choose a base branch
from
Open

[BOLT] Gadget scanner: optionally assume auth traps on failure #139778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
112 changes: 74 additions & 38 deletions bolt/lib/Passes/PAuthGadgetScanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "bolt/Passes/PAuthGadgetScanner.h"
#include "bolt/Core/ParallelUtilities.h"
#include "bolt/Passes/DataflowAnalysis.h"
#include "bolt/Utils/CommandLineOpts.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/MC/MCInst.h"
Expand All @@ -26,6 +27,11 @@ namespace llvm {
namespace bolt {
namespace PAuthGadgetScanner {

static cl::opt<bool> AuthTrapsOnFailure(
"auth-traps-on-failure",
cl::desc("Assume authentication instructions always trap on failure"),
cl::cat(opts::BinaryAnalysisCategory));

[[maybe_unused]] static void traceInst(const BinaryContext &BC, StringRef Label,
const MCInst &MI) {
dbgs() << " " << Label << ": ";
Expand Down Expand Up @@ -364,6 +370,34 @@ class SrcSafetyAnalysis {
return Clobbered;
}

std::optional<MCPhysReg> getRegMadeTrustedByChecking(const MCInst &Inst,
SrcState Cur) const {
// This functions cannot return multiple registers. This is never the case
// on AArch64.
std::optional<MCPhysReg> RegCheckedByInst =
BC.MIB->getAuthCheckedReg(Inst, /*MayOverwrite=*/false);
if (RegCheckedByInst && Cur.SafeToDerefRegs[*RegCheckedByInst])
return *RegCheckedByInst;

auto It = CheckerSequenceInfo.find(&Inst);
if (It == CheckerSequenceInfo.end())
return std::nullopt;

MCPhysReg RegCheckedBySequence = It->second.first;
const MCInst *FirstCheckerInst = It->second.second;

// FirstCheckerInst should belong to the same basic block (see the
// assertion in DataflowSrcSafetyAnalysis::run()), meaning it was
// deterministically processed a few steps before this instruction.
const SrcState &StateBeforeChecker = getStateBefore(*FirstCheckerInst);

// The sequence checks the register, but it should be authenticated before.
if (!StateBeforeChecker.SafeToDerefRegs[RegCheckedBySequence])
return std::nullopt;

return RegCheckedBySequence;
}

// Returns all registers that can be treated as if they are written by an
// authentication instruction.
SmallVector<MCPhysReg> getRegsMadeSafeToDeref(const MCInst &Point,
Expand All @@ -386,18 +420,38 @@ class SrcSafetyAnalysis {
Regs.push_back(DstAndSrc->first);
}

// Make sure explicit checker sequence keeps register safe-to-dereference
// when the register would be clobbered according to the regular rules:
//
// ; LR is safe to dereference here
// mov x16, x30 ; start of the sequence, LR is s-t-d right before
// xpaclri ; clobbers LR, LR is not safe anymore
// cmp x30, x16
// b.eq 1f ; end of the sequence: LR is marked as trusted
// brk 0x1234
// 1:
// ; at this point LR would be marked as trusted,
// ; but not safe-to-dereference
//
// or even just
//
// ; X1 is safe to dereference here
// ldr x0, [x1, #8]!
// ; X1 is trusted here, but it was clobbered due to address write-back
if (auto CheckedReg = getRegMadeTrustedByChecking(Point, Cur))
Regs.push_back(*CheckedReg);

return Regs;
}

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

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

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

if (CheckerSequenceInfo.contains(&Point)) {
MCPhysReg CheckedReg;
const MCInst *FirstCheckerInst;
std::tie(CheckedReg, FirstCheckerInst) = CheckerSequenceInfo.at(&Point);

// FirstCheckerInst should belong to the same basic block (see the
// assertion in DataflowSrcSafetyAnalysis::run()), meaning it was
// deterministically processed a few steps before this instruction.
const SrcState &StateBeforeChecker = getStateBefore(*FirstCheckerInst);
if (StateBeforeChecker.SafeToDerefRegs[CheckedReg])
Regs.push_back(CheckedReg);
}

// ... a safe address can be materialized, or
if (auto NewAddrReg = BC.MIB->getMaterializedAddressRegForPtrAuth(Point))
Regs.push_back(*NewAddrReg);
Expand Down Expand Up @@ -463,28 +504,11 @@ class SrcSafetyAnalysis {
BitVector Clobbered = getClobberedRegs(Point);
SmallVector<MCPhysReg> NewSafeToDerefRegs =
getRegsMadeSafeToDeref(Point, Cur);
SmallVector<MCPhysReg> NewTrustedRegs = getRegsMadeTrusted(Point, Cur);

// Ideally, being trusted is a strictly stronger property than being
// safe-to-dereference. To simplify the computation of Next state, enforce
// this for NewSafeToDerefRegs and NewTrustedRegs. Additionally, this
// fixes the properly for "cumulative" register states in tricky cases
// like the following:
//
// ; LR is safe to dereference here
// mov x16, x30 ; start of the sequence, LR is s-t-d right before
// xpaclri ; clobbers LR, LR is not safe anymore
// cmp x30, x16
// b.eq 1f ; end of the sequence: LR is marked as trusted
// brk 0x1234
// 1:
// ; at this point LR would be marked as trusted,
// ; but not safe-to-dereference
//
for (auto TrustedReg : NewTrustedRegs) {
if (!is_contained(NewSafeToDerefRegs, TrustedReg))
NewSafeToDerefRegs.push_back(TrustedReg);
}
// If authentication instructions trap on failure, safe-to-dereference
// registers are always trusted.
SmallVector<MCPhysReg> NewTrustedRegs =
AuthTrapsOnFailure ? NewSafeToDerefRegs
: getRegsMadeTrusted(Point, Cur);

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

// Being trusted is a strictly stronger property than being
// safe-to-dereference.
assert(!Next.TrustedRegs.test(Next.SafeToDerefRegs) &&
"SafeToDerefRegs should contain all TrustedRegs");

Comment on lines +548 to +552
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BitVector::test(const BitVector &) name looks rather ambiguous to me, but here is its definition: BitVector.h.

return Next;
}

Expand Down Expand Up @@ -1136,6 +1165,11 @@ class DataflowDstSafetyAnalysis
}

void run() override {
// As long as DstSafetyAnalysis is only computed to detect authentication
// oracles, it is a waste of time to compute it when authentication
// instructions are known to always trap on failure.
assert(!AuthTrapsOnFailure &&
"DstSafetyAnalysis is useless with faulting auth");
for (BinaryBasicBlock &BB : Func) {
if (auto CheckerInfo = BC.MIB->getAuthCheckedReg(BB)) {
LLVM_DEBUG({
Expand Down Expand Up @@ -1587,6 +1621,8 @@ void FunctionAnalysisContext::findUnsafeDefs(
SmallVector<PartialReport<MCPhysReg>> &Reports) {
if (PacRetGadgetsOnly)
return;
if (AuthTrapsOnFailure)
return;

auto Analysis = DstSafetyAnalysis::create(BF, AllocatorId, {});
LLVM_DEBUG({ dbgs() << "Running dst register safety analysis...\n"; });
Expand Down
1 change: 1 addition & 0 deletions bolt/test/binary-analysis/AArch64/cmdline-args.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ HELP-NEXT: OPTIONS:
HELP-EMPTY:
HELP-NEXT: BinaryAnalysis options:
HELP-EMPTY:
HELP-NEXT: --auth-traps-on-failure - Assume authentication instructions always trap on failure
HELP-NEXT: --scanners=<value> - which gadget scanners to run
HELP-NEXT: =pacret - pac-ret: return address protection (subset of "pauth")
HELP-NEXT: =pauth - All Pointer Authentication scanners
Expand Down
6 changes: 4 additions & 2 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
@@ -1,13 +1,15 @@
// RUN: %clang %cflags -march=armv8.3-a %s -o %t.exe
// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s
// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s
// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s
// RUN: llvm-bolt-binary-analysis --scanners=pauth --auth-traps-on-failure %t.exe 2>&1 | FileCheck -check-prefix=FPAC %s
// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s

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

// PACRET-NOT: authentication oracle found in function
// FPAC-NOT: authentication oracle found in function

.text

Expand Down
5 changes: 3 additions & 2 deletions bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// RUN: %clang %cflags -march=armv8.3-a %s -o %t.exe
// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s
// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s
// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s
// RUN: llvm-bolt-binary-analysis --scanners=pauth --auth-traps-on-failure %t.exe 2>&1 | FileCheck %s
// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s

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

Expand Down
Loading
Loading