Skip to content

Commit fb70474

Browse files
committed
analyzer.ts: fix dependencies through side effects
Commit 2d07fba enhanced dependency tracking with information about value changes reported by the verifier. This has broken an assumption in `getMemSlotDependencies` that only one slot can be written by an instruction. With side effects, it can be many. This lead to incorrect dependency chain being displayed in some cases. Fix this by distinguishing and additional case in `getMemSlotDependencies`, which now handles three: * an actual update such as from `+=` instruction * an actual write, which can be detected by checking `writes` set of a `BpfInstruction` * a side effect, which can only be noticed via value change in the BpfState (this is the new case)
1 parent 59dcbc9 commit fb70474

File tree

2 files changed

+30
-5
lines changed

2 files changed

+30
-5
lines changed

src/analyzer.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
BpfState,
33
BpfValue,
4+
getMemSlotDependencies,
45
initialBpfState,
56
processRawLines,
67
VerifierLogState,
@@ -659,4 +660,26 @@ Func#123 ('my_global_func') is global and assumed valid.
659660
expect(s.values.get("r6")?.value).toBe("42");
660661
});
661662
});
663+
664+
describe("tracks side effect dependencies", () => {
665+
const rawLog = `
666+
3: (85) call bpf_obj_new_impl#54651 ; R0_w=ptr_or_null_node_data(id=2,ref_obj_id=2) refs=2
667+
4: (bf) r6 = r0 ; R0_w=ptr_or_null_node_data(id=2,ref_obj_id=2) R6_w=ptr_or_null_node_data(id=2,ref_obj_id=2) refs=2
668+
6: (15) if r6 == 0x0 goto pc+104 ; R6_w=ptr_node_data(ref_obj_id=2) refs=2
669+
42: (85) call bpf_rbtree_add_impl#54894 ; R0_w=scalar() R6=ptr_node_data(non_own_ref) R7=2 R8=ptr_node_data(ref_obj_id=4) R9=ptr_node_data(ref_obj_id=6) R10=fp0 refs=4,6
670+
99: (55) if r0 != 0x0 goto pc+13 113: R0_w=ptr_node_data(non_own_ref,off=16) R6=scalar() R7=5 R8=scalar() R9=scalar() R10=fp0
671+
117: (18) r1 = 0xff434b28008e3de8 ; R1_w=map_value(map=.data.A,ks=4,vs=72,off=16)
672+
119: (85) call bpf_spin_unlock#94 ;
673+
120: (79) r7 = *(u64 *)(r6 +8)
674+
`;
675+
const logState = getVerifierLogState(rawLog);
676+
it("120: (79) r7 = *(u64 *)(r6 +8)", () => {
677+
const idx = 7;
678+
const s = logState.bpfStates[idx];
679+
expect(s.idx).toBe(7);
680+
expect(s.pc).toBe(120);
681+
const r6Deps = getMemSlotDependencies(logState, idx, "r6");
682+
expect(r6Deps).toEqual(new Set<number>([4, 3, 2, 1, 0]));
683+
});
684+
});
662685
});

src/analyzer.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,8 @@ export function getMemSlotDependencies(
485485
if (!effect) return deps;
486486

487487
const depIdx = bpfState.lastKnownWrites.get(memSlotId);
488-
if (!depIdx || lines[depIdx].type !== ParsedLineType.INSTRUCTION) return deps;
488+
if (depIdx === undefined || lines[depIdx].type !== ParsedLineType.INSTRUCTION)
489+
return deps;
489490

490491
const depIns = lines[depIdx].bpfIns;
491492
const nReads = depIns?.reads?.length;
@@ -503,11 +504,12 @@ export function getMemSlotDependencies(
503504

504505
deps = getMemSlotDependencies(verifierLogState, prevDepIdx, memSlotId);
505506
}
506-
} else if (
507-
nReads === 1 &&
508-
(depIdx !== bpfState.idx || depIns.reads[0] !== memSlotId)
509-
) {
507+
} else if (nReads === 1 && depIns.writes?.includes(memSlotId)) {
508+
// following a direct write from a singular source
510509
deps = getMemSlotDependencies(verifierLogState, depIdx, depIns.reads[0]);
510+
} else if (depIdx !== selectedLine) {
511+
// following a side effect
512+
deps = getMemSlotDependencies(verifierLogState, depIdx, memSlotId);
511513
}
512514

513515
deps.add(depIdx);

0 commit comments

Comments
 (0)