Skip to content

Commit 3be6366

Browse files
SC llvm teamSC llvm team
SC llvm team
authored and
SC llvm team
committed
Merged main:0fc7aec34939 into origin/amd-gfx:bc9dded03b0b
Local branch origin/amd-gfx bc9dded Merged main:be6ccc98f382 into origin/amd-gfx:943bd63b166f Remote branch main 0fc7aec [BOLT] Gadget scanner: detect address materialization and arithmetic (llvm#132540)
2 parents bc9dded + 0fc7aec commit 3be6366

File tree

16 files changed

+480
-102
lines changed

16 files changed

+480
-102
lines changed

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,41 @@ class MCPlusBuilder {
587587
return getNoRegister();
588588
}
589589

590+
/// Returns the register containing an address safely materialized by `Inst`
591+
/// under the Pointer Authentication threat model.
592+
///
593+
/// Returns the register `Inst` writes to if:
594+
/// 1. the register is a materialized address, and
595+
/// 2. the register has been materialized safely, i.e. cannot be attacker-
596+
/// controlled, under the Pointer Authentication threat model.
597+
///
598+
/// If the instruction does not write to any register satisfying the above
599+
/// two conditions, NoRegister is returned.
600+
///
601+
/// The Pointer Authentication threat model assumes an attacker is able to
602+
/// modify any writable memory, but not executable code (due to W^X).
603+
virtual MCPhysReg
604+
getMaterializedAddressRegForPtrAuth(const MCInst &Inst) const {
605+
llvm_unreachable("not implemented");
606+
return getNoRegister();
607+
}
608+
609+
/// Analyzes if this instruction can safely perform address arithmetics
610+
/// under Pointer Authentication threat model.
611+
///
612+
/// If an (OutReg, InReg) pair is returned, then after Inst is executed,
613+
/// OutReg is as trusted as InReg is.
614+
///
615+
/// The arithmetic instruction is considered safe if OutReg is not attacker-
616+
/// controlled, provided InReg and executable code are not. Please note that
617+
/// registers other than InReg as well as the contents of memory which is
618+
/// writable by the process should be considered attacker-controlled.
619+
virtual std::optional<std::pair<MCPhysReg, MCPhysReg>>
620+
analyzeAddressArithmeticsForPtrAuth(const MCInst &Inst) const {
621+
llvm_unreachable("not implemented");
622+
return std::make_pair(getNoRegister(), getNoRegister());
623+
}
624+
590625
virtual bool isTerminator(const MCInst &Inst) const;
591626

592627
virtual bool isNoop(const MCInst &Inst) const {

bolt/lib/Passes/PAuthGadgetScanner.cpp

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,49 @@ class PacRetAnalysis
335335
});
336336
}
337337

338+
BitVector getClobberedRegs(const MCInst &Point) const {
339+
BitVector Clobbered(NumRegs, false);
340+
// Assume a call can clobber all registers, including callee-saved
341+
// registers. There's a good chance that callee-saved registers will be
342+
// saved on the stack at some point during execution of the callee.
343+
// Therefore they should also be considered as potentially modified by an
344+
// attacker/written to.
345+
// Also, not all functions may respect the AAPCS ABI rules about
346+
// caller/callee-saved registers.
347+
if (BC.MIB->isCall(Point))
348+
Clobbered.set();
349+
else
350+
BC.MIB->getClobberedRegs(Point, Clobbered);
351+
return Clobbered;
352+
}
353+
354+
// Returns all registers that can be treated as if they are written by an
355+
// authentication instruction.
356+
SmallVector<MCPhysReg> getRegsMadeSafeToDeref(const MCInst &Point,
357+
const State &Cur) const {
358+
SmallVector<MCPhysReg> Regs;
359+
const MCPhysReg NoReg = BC.MIB->getNoRegister();
360+
361+
// A signed pointer can be authenticated, or
362+
ErrorOr<MCPhysReg> AutReg = BC.MIB->getAuthenticatedReg(Point);
363+
if (AutReg && *AutReg != NoReg)
364+
Regs.push_back(*AutReg);
365+
366+
// ... a safe address can be materialized, or
367+
MCPhysReg NewAddrReg = BC.MIB->getMaterializedAddressRegForPtrAuth(Point);
368+
if (NewAddrReg != NoReg)
369+
Regs.push_back(NewAddrReg);
370+
371+
// ... an address can be updated in a safe manner, producing the result
372+
// which is as trusted as the input address.
373+
if (auto DstAndSrc = BC.MIB->analyzeAddressArithmeticsForPtrAuth(Point)) {
374+
if (Cur.SafeToDerefRegs[DstAndSrc->second])
375+
Regs.push_back(DstAndSrc->first);
376+
}
377+
378+
return Regs;
379+
}
380+
338381
State computeNext(const MCInst &Point, const State &Cur) {
339382
PacStatePrinter P(BC);
340383
LLVM_DEBUG({
@@ -355,37 +398,35 @@ class PacRetAnalysis
355398
return State();
356399
}
357400

401+
// First, compute various properties of the instruction, taking the state
402+
// before its execution into account, if necessary.
403+
404+
BitVector Clobbered = getClobberedRegs(Point);
405+
SmallVector<MCPhysReg> NewSafeToDerefRegs =
406+
getRegsMadeSafeToDeref(Point, Cur);
407+
408+
// Then, compute the state after this instruction is executed.
358409
State Next = Cur;
359-
BitVector Clobbered(NumRegs, false);
360-
// Assume a call can clobber all registers, including callee-saved
361-
// registers. There's a good chance that callee-saved registers will be
362-
// saved on the stack at some point during execution of the callee.
363-
// Therefore they should also be considered as potentially modified by an
364-
// attacker/written to.
365-
// Also, not all functions may respect the AAPCS ABI rules about
366-
// caller/callee-saved registers.
367-
if (BC.MIB->isCall(Point))
368-
Clobbered.set();
369-
else
370-
BC.MIB->getClobberedRegs(Point, Clobbered);
410+
371411
Next.SafeToDerefRegs.reset(Clobbered);
372412
// Keep track of this instruction if it writes to any of the registers we
373413
// need to track that for:
374414
for (MCPhysReg Reg : RegsToTrackInstsFor.getRegisters())
375415
if (Clobbered[Reg])
376416
lastWritingInsts(Next, Reg) = {&Point};
377417

378-
ErrorOr<MCPhysReg> AutReg = BC.MIB->getAuthenticatedReg(Point);
379-
if (AutReg && *AutReg != BC.MIB->getNoRegister()) {
380-
// The sub-registers of *AutReg are also trusted now, but not its
381-
// super-registers (as they retain untrusted register units).
382-
BitVector AuthenticatedSubregs =
383-
BC.MIB->getAliases(*AutReg, /*OnlySmaller=*/true);
384-
for (MCPhysReg Reg : AuthenticatedSubregs.set_bits()) {
385-
Next.SafeToDerefRegs.set(Reg);
386-
if (RegsToTrackInstsFor.isTracked(Reg))
387-
lastWritingInsts(Next, Reg).clear();
388-
}
418+
// After accounting for clobbered registers in general, override the state
419+
// according to authentication and other *special cases* of clobbering.
420+
421+
// The sub-registers are also safe-to-dereference now, but not their
422+
// super-registers (as they retain untrusted register units).
423+
BitVector NewSafeSubregs(NumRegs);
424+
for (MCPhysReg SafeReg : NewSafeToDerefRegs)
425+
NewSafeSubregs |= BC.MIB->getAliases(SafeReg, /*OnlySmaller=*/true);
426+
for (MCPhysReg Reg : NewSafeSubregs.set_bits()) {
427+
Next.SafeToDerefRegs.set(Reg);
428+
if (RegsToTrackInstsFor.isTracked(Reg))
429+
lastWritingInsts(Next, Reg).clear();
389430
}
390431

391432
LLVM_DEBUG({

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,43 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
304304
}
305305
}
306306

307+
MCPhysReg
308+
getMaterializedAddressRegForPtrAuth(const MCInst &Inst) const override {
309+
switch (Inst.getOpcode()) {
310+
case AArch64::ADR:
311+
case AArch64::ADRP:
312+
// These instructions produce an address value based on the information
313+
// encoded into the instruction itself (which should reside in a read-only
314+
// code memory) and the value of PC register (that is, the location of
315+
// this instruction), so the produced value is not attacker-controlled.
316+
return Inst.getOperand(0).getReg();
317+
default:
318+
return getNoRegister();
319+
}
320+
}
321+
322+
std::optional<std::pair<MCPhysReg, MCPhysReg>>
323+
analyzeAddressArithmeticsForPtrAuth(const MCInst &Inst) const override {
324+
switch (Inst.getOpcode()) {
325+
default:
326+
return std::nullopt;
327+
case AArch64::ADDXri:
328+
case AArch64::SUBXri:
329+
// The immediate addend is encoded into the instruction itself, so it is
330+
// not attacker-controlled under Pointer Authentication threat model.
331+
return std::make_pair(Inst.getOperand(0).getReg(),
332+
Inst.getOperand(1).getReg());
333+
case AArch64::ORRXrs:
334+
// "mov Xd, Xm" is equivalent to "orr Xd, XZR, Xm, lsl #0"
335+
if (Inst.getOperand(1).getReg() != AArch64::XZR ||
336+
Inst.getOperand(3).getImm() != 0)
337+
return std::nullopt;
338+
339+
return std::make_pair(Inst.getOperand(0).getReg(),
340+
Inst.getOperand(2).getReg());
341+
}
342+
}
343+
307344
bool isADRP(const MCInst &Inst) const override {
308345
return Inst.getOpcode() == AArch64::ADRP;
309346
}

bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -141,24 +141,9 @@ f_nonx30_ret_ok:
141141
stp x29, x30, [sp, #-16]!
142142
mov x29, sp
143143
bl g
144-
add x0, x0, #3
145144
ldp x29, x30, [sp], #16
146-
// FIXME: Should the scanner understand that an authenticated register (below x30,
147-
// after the autiasp instruction), is OK to be moved to another register
148-
// and then that register being used to return?
149-
// This respects that pac-ret hardening intent, but the scanner currently
150-
// will produce a false positive for this.
151-
// Is it worthwhile to make the scanner more complex for this case?
152-
// So far, scanning many millions of instructions across a linux distro,
153-
// I haven't encountered such an example.
154-
// The ".if 0" block below tests this case and currently fails.
155-
.if 0
156145
autiasp
157146
mov x16, x30
158-
.else
159-
mov x16, x30
160-
autia x16, sp
161-
.endif
162147
// CHECK-NOT: function f_nonx30_ret_ok
163148
ret x16
164149
.size f_nonx30_ret_ok, .-f_nonx30_ret_ok

0 commit comments

Comments
 (0)