Skip to content

Commit cc1d797

Browse files
committed
validate direct branch target
1 parent d30bd27 commit cc1d797

File tree

2 files changed

+47
-0
lines changed

2 files changed

+47
-0
lines changed

bolt/include/bolt/Core/BinaryFunction.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2236,6 +2236,13 @@ class BinaryFunction {
22362236
/// it is probably another function.
22372237
bool isSymbolValidInScope(const SymbolRef &Symbol, uint64_t SymbolSize) const;
22382238

2239+
/// Validates if the target of a direct branch/call is a valid
2240+
/// executable instruction.
2241+
/// Return true if the target is valid, false otherwise.
2242+
bool validateBranchTarget(uint64_t TargetAddress,
2243+
uint64_t AbsoluteInstrAddr,
2244+
const ArrayRef<uint8_t> &CurrentFunctionData);
2245+
22392246
/// Disassemble function from raw data.
22402247
/// If successful, this function will populate the list of instructions
22412248
/// for this function together with offsets from the function start

bolt/lib/Core/BinaryFunction.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,41 @@ BinaryFunction::disassembleInstructionAtOffset(uint64_t Offset) const {
12831283
return std::nullopt;
12841284
}
12851285

1286+
bool BinaryFunction::validateBranchTarget(
1287+
uint64_t TargetAddress, uint64_t AbsoluteInstrAddr,
1288+
const ArrayRef<uint8_t> &CurrentFunctionData) {
1289+
if (auto *TargetFunc =
1290+
BC.getBinaryFunctionContainingAddress(TargetAddress)) {
1291+
const uint64_t TargetOffset = TargetAddress - TargetFunc->getAddress();
1292+
ArrayRef<uint8_t> TargetFunctionData;
1293+
// Check if the target address is within the current function.
1294+
if (TargetFunc == this) {
1295+
TargetFunctionData = CurrentFunctionData;
1296+
} else {
1297+
// external call/branch, fetch the binary data for target
1298+
ErrorOr<ArrayRef<uint8_t>> TargetDataOrErr = TargetFunc->getData();
1299+
assert(TargetDataOrErr && "function data is not available");
1300+
TargetFunctionData = *TargetDataOrErr;
1301+
}
1302+
1303+
MCInst TargetInst;
1304+
uint64_t TargetInstSize;
1305+
if (!BC.SymbolicDisAsm->getInstruction(
1306+
TargetInst, TargetInstSize, TargetFunctionData.slice(TargetOffset),
1307+
TargetAddress, nulls())) {
1308+
// If the target address cannot be disassembled well,
1309+
// it implies a corrupted control flow.
1310+
BC.errs() << "BOLT-WARNING: direct branch/call at 0x"
1311+
<< Twine::utohexstr(AbsoluteInstrAddr) << " in function "
1312+
<< *this << " targets an invalid instruction at 0x"
1313+
<< Twine::utohexstr(TargetAddress) << "\n";
1314+
return false;
1315+
}
1316+
}
1317+
1318+
return true;
1319+
}
1320+
12861321
Error BinaryFunction::disassemble() {
12871322
NamedRegionTimer T("disassemble", "Disassemble function", "buildfuncs",
12881323
"Build Binary Functions", opts::TimeBuild);
@@ -1396,6 +1431,11 @@ Error BinaryFunction::disassemble() {
13961431
uint64_t TargetAddress = 0;
13971432
if (MIB->evaluateBranch(Instruction, AbsoluteInstrAddr, Size,
13981433
TargetAddress)) {
1434+
if (!validateBranchTarget(TargetAddress, AbsoluteInstrAddr,
1435+
FunctionData)) {
1436+
setIgnored();
1437+
break;
1438+
}
13991439
// Check if the target is within the same function. Otherwise it's
14001440
// a call, possibly a tail call.
14011441
//

0 commit comments

Comments
 (0)