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>
