Skip to content

Commit 5d06789

Browse files
committed
[BOLT] Gadget scanner: detect untrusted LR before tail call
Implement the detection of tail calls performed with untrusted link register, which violates the assumption made on entry to every function. Unlike other pauth gadgets, this one involves some amount of guessing which branch instructions should be checked as tail calls.
1 parent 75b3070 commit 5d06789

File tree

2 files changed

+677
-0
lines changed

2 files changed

+677
-0
lines changed

bolt/lib/Passes/PAuthGadgetScanner.cpp

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,83 @@ shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
13191319
return make_gadget_report(RetKind, Inst, *RetReg);
13201320
}
13211321

1322+
/// While BOLT already marks some of the branch instructions as tail calls,
1323+
/// this function tries to improve the coverage by including less obvious cases
1324+
/// when it is possible to do without introducing too many false positives.
1325+
static bool shouldAnalyzeTailCallInst(const BinaryContext &BC,
1326+
const BinaryFunction &BF,
1327+
const MCInstReference &Inst) {
1328+
// Some BC.MIB->isXYZ(Inst) methods simply delegate to MCInstrDesc::isXYZ()
1329+
// (such as isBranch at the time of writing this comment), some don't (such
1330+
// as isCall). For that reason, call MCInstrDesc's methods explicitly when
1331+
// it is important.
1332+
const MCInstrDesc &Desc =
1333+
BC.MII->get(static_cast<const MCInst &>(Inst).getOpcode());
1334+
// Tail call should be a branch (but not necessarily an indirect one).
1335+
if (!Desc.isBranch())
1336+
return false;
1337+
1338+
// Always analyze the branches already marked as tail calls by BOLT.
1339+
if (BC.MIB->isTailCall(Inst))
1340+
return true;
1341+
1342+
// Try to also check the branches marked as "UNKNOWN CONTROL FLOW" - the
1343+
// below is a simplified condition from BinaryContext::printInstruction.
1344+
bool IsUnknownControlFlow =
1345+
BC.MIB->isIndirectBranch(Inst) && !BC.MIB->getJumpTable(Inst);
1346+
1347+
if (BF.hasCFG() && IsUnknownControlFlow)
1348+
return true;
1349+
1350+
return false;
1351+
}
1352+
1353+
static std::optional<PartialReport<MCPhysReg>>
1354+
shouldReportUnsafeTailCall(const BinaryContext &BC, const BinaryFunction &BF,
1355+
const MCInstReference &Inst, const SrcState &S) {
1356+
static const GadgetKind UntrustedLRKind(
1357+
"untrusted link register found before tail call");
1358+
1359+
if (!shouldAnalyzeTailCallInst(BC, BF, Inst))
1360+
return std::nullopt;
1361+
1362+
// Not only the set of registers returned by getTrustedLiveInRegs() can be
1363+
// seen as a reasonable target-independent _approximation_ of "the LR", these
1364+
// are *exactly* those registers used by SrcSafetyAnalysis to initialize the
1365+
// set of trusted registers on function entry.
1366+
// Thus, this function basically checks that the precondition expected to be
1367+
// imposed by a function call instruction (which is hardcoded into the target-
1368+
// specific getTrustedLiveInRegs() function) is also respected on tail calls.
1369+
SmallVector<MCPhysReg> RegsToCheck = BC.MIB->getTrustedLiveInRegs();
1370+
LLVM_DEBUG({
1371+
traceInst(BC, "Found tail call inst", Inst);
1372+
traceRegMask(BC, "Trusted regs", S.TrustedRegs);
1373+
});
1374+
1375+
// In musl on AArch64, the _start function sets LR to zero and calls the next
1376+
// stage initialization function at the end, something along these lines:
1377+
//
1378+
// _start:
1379+
// mov x30, #0
1380+
// ; ... other initialization ...
1381+
// b _start_c ; performs "exit" system call at some point
1382+
//
1383+
// As this would produce a false positive for every executable linked with
1384+
// such libc, ignore tail calls performed by ELF entry function.
1385+
if (BC.StartFunctionAddress &&
1386+
*BC.StartFunctionAddress == Inst.getFunction()->getAddress()) {
1387+
LLVM_DEBUG({ dbgs() << " Skipping tail call in ELF entry function.\n"; });
1388+
return std::nullopt;
1389+
}
1390+
1391+
// Returns at most one report per instruction - this is probably OK...
1392+
for (auto Reg : RegsToCheck)
1393+
if (!S.TrustedRegs[Reg])
1394+
return make_gadget_report(UntrustedLRKind, Inst, Reg);
1395+
1396+
return std::nullopt;
1397+
}
1398+
13221399
static std::optional<PartialReport<MCPhysReg>>
13231400
shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
13241401
const SrcState &S) {
@@ -1478,6 +1555,9 @@ void FunctionAnalysisContext::findUnsafeUses(
14781555
if (PacRetGadgetsOnly)
14791556
return;
14801557

1558+
if (auto Report = shouldReportUnsafeTailCall(BC, BF, Inst, S))
1559+
Reports.push_back(*Report);
1560+
14811561
if (auto Report = shouldReportCallGadget(BC, Inst, S))
14821562
Reports.push_back(*Report);
14831563
if (auto Report = shouldReportSigningOracle(BC, Inst, S))

0 commit comments

Comments
 (0)