Skip to content

Commit 5d549bc

Browse files
committed
analyzer.ts: Indirect stack access evaluation
Teach analyzer to evaluate indirect stack access when computing the array of BpfState objects. This allows to track stack loads and stores done through a register other than r10. For example: *(u64 *)(r1 +0) = r8 If at this point r1 = fp-24, then the value of r8 is written there. So far bpfvv only could detect this from verifier-provided value changes. Now it actually checks for the value of r1 at the point of a store, and can detect a write to fp-24 even if value expression is absent from the log.
1 parent e7847a1 commit 5d549bc

File tree

4 files changed

+119
-12
lines changed

4 files changed

+119
-12
lines changed

src/analyzer.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,4 +601,62 @@ Func#123 ('my_global_func') is global and assumed valid.
601601
expect(s.lastKnownWrites.get("r7")).toBe(5);
602602
});
603603
});
604+
605+
// The following tow tests aim to verify that the analyzer correctly
606+
// identifies an indirect access to a stack slot
607+
// note that typically verifier would print an appropriate expression after ;
608+
// but analyzer must be able to infer the access independently
609+
describe("evaluates indirect stack store", () => {
610+
const rawLog = `
611+
525: (bf) r1 = r10 ; R1_w=fp0 R10=fp0
612+
526: (07) r1 += -24 ; R1_w=fp-24
613+
527: (79) r2 = *(u64 *)(r10 -56) ; R2_w=0 R10=fp0 fp-56=0
614+
528: (0f) r1 += r2
615+
529: R1_w=fp-24 R2_w=0
616+
529: (7b) *(u64 *)(r1 +0) = r8 ; R1_w=fp-24 R8=scalar(id=102)
617+
530: (79) r6 = *(u64 *)(r10 -24)
618+
`;
619+
const { bpfStates } = getVerifierLogState(rawLog);
620+
const val = "scalar(id=102)";
621+
it("*(u64 *)(r1 +0) = r8", () => {
622+
const s = bpfStates[5];
623+
expect(s.idx).toBe(5);
624+
expect(s.pc).toBe(529);
625+
expect(s.values.get("r8")?.value).toBe(val);
626+
expect(s.values.get("fp-24")?.value).toBe(val);
627+
});
628+
it("r6 = *(u64 *)(r10 -24)", () => {
629+
const s = bpfStates[6];
630+
expect(s.idx).toBe(6);
631+
expect(s.pc).toBe(530);
632+
expect(s.values.get("fp-24")?.value).toBe(val);
633+
expect(s.values.get("r6")?.value).toBe(val);
634+
});
635+
});
636+
637+
describe("evaluates indirect stack load", () => {
638+
const rawLog = `
639+
524: (7b) *(u64 *)(r10 -24) = r9 ; fp-24_w=42 R9=42
640+
525: (bf) r1 = r10 ; R1_w=fp0 R10=fp0
641+
526: (07) r1 += -16 ; R1_w=fp-16
642+
527: (18) r2 = 0 ; R2_w=0 R10=fp0
643+
528: (0f) r1 += r2 ; R1_w=fp-16 R2_w=0
644+
530: (79) r6 = *(u64 *)(r1 -8)
645+
`;
646+
const { bpfStates } = getVerifierLogState(rawLog);
647+
it("*(u64 *)(r10 -24) = r9", () => {
648+
const s = bpfStates[0];
649+
expect(s.idx).toBe(0);
650+
expect(s.pc).toBe(524);
651+
expect(s.values.get("r9")?.value).toBe("42");
652+
expect(s.values.get("fp-24")?.value).toBe("42");
653+
});
654+
it("r6 = *(u64 *)(r1 -8)", () => {
655+
const s = bpfStates[5];
656+
expect(s.idx).toBe(5);
657+
expect(s.pc).toBe(530);
658+
expect(s.values.get("fp-24")?.value).toBe("42");
659+
expect(s.values.get("r6")?.value).toBe("42");
660+
});
661+
});
604662
});

src/analyzer.ts

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import {
1313
GlobalFuncValidInfo,
1414
InstructionLine,
1515
BpfStateExprsInfo,
16+
OperandType,
17+
BpfOperand,
1618
} from "./parser";
1719
import { siblingInsLine } from "./utils";
1820

@@ -233,22 +235,32 @@ function nextBpfState(
233235
}
234236

235237
newState = copyBpfState(bpfState);
238+
239+
function srcValue(): string {
240+
if (ins.kind === BpfInstructionKind.ALU) {
241+
return newState.values.get(ins.src.id)?.value || "";
242+
} else {
243+
return "";
244+
}
245+
}
246+
236247
let effects = new Map<string, Effect>();
237248
for (const id of line.bpfIns?.reads || []) {
238249
effects.set(id, Effect.READ);
239250
}
251+
240252
for (const id of line.bpfIns?.writes || []) {
241253
if (effects.has(id)) effects.set(id, Effect.UPDATE);
242254
else effects.set(id, Effect.WRITE);
243-
const val = makeValue("", effects.get(id));
255+
256+
const val = makeValue(srcValue(), effects.get(id));
244257
if (val.effect === Effect.UPDATE)
245258
val.prevValue = newState.values.get(id)?.value;
246259
newState.values.set(id, val);
247260
newState.lastKnownWrites.set(id, line.idx);
248261
}
249262

250-
// If verifier reported a particular expr, it overrides any values we may have computed so far
251-
// This means, for example, that conditions can be UPDATEs
263+
const verifierReportedValues = new Map<string, string>();
252264
for (const expr of line.bpfStateExprs) {
253265
const val: BpfValue | undefined = newState.values.get(expr.id);
254266
let effect = effects.get(expr.id) || Effect.NONE;
@@ -263,6 +275,45 @@ function nextBpfState(
263275
expr.id,
264276
makeValue(expr.value, effect, val?.prevValue || ""),
265277
);
278+
verifierReportedValues.set(expr.id, expr.value);
279+
}
280+
281+
// Evaluate indirect stack load/store
282+
// For any *(u64 *)(r<i> +<roff>) check the current value of r<i> and
283+
// if it is equal to fp-<soff>, evaluate to fp-(<soff>+<roff>) and
284+
// update the values map accordingly
285+
function fpIdFromMemref(op: BpfOperand): string | null {
286+
if (!op.memref) return null;
287+
const { reg, offset } = op.memref;
288+
const val = newState.values.get(reg)?.value;
289+
const match = val?.match(/fp-([0-9]+)/);
290+
if (match) {
291+
const off = offset - parseInt(match[1]);
292+
return `fp${off}`;
293+
} else {
294+
return null;
295+
}
296+
}
297+
298+
if (ins.kind === BpfInstructionKind.ALU && ins.operator === "=") {
299+
if (ins.dst.type === OperandType.MEM) {
300+
const id = fpIdFromMemref(ins.dst);
301+
if (id) {
302+
const value = verifierReportedValues.get(id) || srcValue();
303+
newState.values.set(id, makeValue(value, Effect.WRITE));
304+
}
305+
}
306+
if (ins.src.type === OperandType.MEM) {
307+
const id = fpIdFromMemref(ins.src);
308+
if (id) {
309+
const value =
310+
verifierReportedValues.get(id) ||
311+
newState.values.get(id)?.value ||
312+
"";
313+
newState.values.set(id, makeValue(value, Effect.READ));
314+
newState.values.set(ins.dst.id, makeValue(value, Effect.WRITE));
315+
}
316+
}
266317
}
267318

268319
setIdxAndPc(newState);

src/components.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ describe("MemSlot", () => {
104104
);
105105
const op = createOp(OperandType.MEM, 15, -38, "MEM");
106106
op.memref = {
107-
address_reg: "r2",
107+
reg: "r2",
108108
offset: 0,
109109
};
110110
render(<MemSlot line={line} op={op} />);

src/parser.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ export type BpfOperand = {
175175
id: string; // r0-r10 for regs, 'fp-off' for stack
176176
size: number;
177177
memref?: {
178-
address_reg: string;
178+
reg: string;
179179
offset: number;
180180
};
181181
location?: RawLineLocation;
@@ -424,17 +424,17 @@ function parseMemoryRef(str: string): BpfOperandPair {
424424
const { match, rest } = consumeRegex(RE_MEMORY_REF, str);
425425
if (!match) return [undefined, rest];
426426
const size = CAST_TO_SIZE.get(match[1]) || 0;
427-
const address_reg = match[2];
427+
const reg = match[2];
428428
const offset = parseInt(match[3], 10);
429429
// We do not currently use memory ids, and they blow up the lastKnownWrites map in the app
430430
// So let's use a dummy id for now, like for immediates
431431
let id = "MEM";
432432
let type = OperandType.MEM;
433-
if (address_reg === "r10") {
433+
if (reg === "r10") {
434434
id = "fp" + offset;
435435
type = OperandType.FP;
436436
}
437-
const op = { id, type, size, memref: { address_reg, offset } };
437+
const op = { id, type, size, memref: { reg, offset } };
438438
return [op, rest];
439439
}
440440

@@ -465,10 +465,8 @@ function collectReads(
465465
): string[] {
466466
const reads = [];
467467
if (operator !== "=") reads.push(dst.id);
468-
if (src.type === OperandType.MEM && src.memref)
469-
reads.push(src.memref.address_reg);
470-
if (dst.type === OperandType.MEM && dst.memref)
471-
reads.push(dst.memref.address_reg);
468+
if (src.type === OperandType.MEM && src.memref) reads.push(src.memref.reg);
469+
if (dst.type === OperandType.MEM && dst.memref) reads.push(dst.memref.reg);
472470
// do not add src to reads if it's a store from immediate value
473471
if (src.type !== OperandType.IMM) reads.push(src.id);
474472
return reads;

0 commit comments

Comments
 (0)