On Thu, Nov 9, 2023 at 9:20 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Wed, 2023-11-08 at 15:11 -0800, Andrii Nakryiko wrote: > > Fix an edge case in __mark_chain_precision() which prematurely stops > > backtracking instructions in a state if it happens that state's first > > and last instruction indexes are the same. This situations doesn't > > necessarily mean that there were no instructions simulated in a state, > > but rather that we starting from the instruction, jumped around a bit, > > and then ended up at the same instruction before checkpointing or > > marking precision. > > > > To distinguish between these two possible situations, we need to consult > > jump history. If it's empty or contain a single record "bridging" parent > > state and first instruction of processed state, then we indeed > > backtracked all instructions in this state. But if history is not empty, > > we are definitely not done yet. > > > > Move this logic inside get_prev_insn_idx() to contain it more nicely. > > Use -ENOENT return code to denote "we are out of instructions" > > situation. > > > > This bug was exposed by verifier_cfg.c's bounded_recursion subtest, once > > Note: verifier_cfg.c should be verifier_loops1.c fixed > > > the next fix in this patch set is applied. > > > > Fixes: b5dc0163d8fd ("bpf: precise scalar_value tracking") > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > Funny how nobody noticed this bug for so long, I looked at exactly > this code today while going through your other patch-set and no alarm > bells rang in my head. tell me about it, I spent 3 hours with gdb and tons of extra verifier logging before I found the issue > > I think that this case needs a dedicated test case that would check > precision tracking log. ok, will add > > Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>