Re: [PATCH bpf-next v2 03/13] bpf: track low-32 scalar equality across zero-extending movs

[bot+bpf-ci]

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index afb1e5628698..f1b01059c5da 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0ca229f6e7ac..58e788f53ae5 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -17115,6 +17126,23 @@ static void collect_linked_regs(struct bpf_verifier_env *env,
>  	}
>  }
>
> +/*
> + * Set @reg to the zero-extension of @known_reg's low 32 bits: it shares those
> + * bits and its high half is zero. Copy the base to keep its precise low-32
> + * tnum, then re-apply the zext_32_to_64() the 32-bit mov itself used.
> + * @reg->id and ->delta already equal @known_reg's; only ->subreg is its own.
> + */
> +static void reconstruct_zext32(struct bpf_reg_state *reg,
> +			       struct bpf_reg_state *known_reg)
> +{
> +	enum bpf_subreg subreg = reg->subreg;
> +
> +	*reg = *known_reg;
> +	reg->subreg = subreg;
> +	zext_32_to_64(reg);
> +	reg_bounds_sync(reg);
> +}
> +
>  /* For all R in linked_regs, copy known_reg range into R
>   * if R->id == known_reg->id.
>   */
> @@ -17134,6 +17162,27 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s
>  			continue;
>  		if (reg->id != known_reg->id)
>  			continue;
> +		/*
> +		 * A ->subreg register shares only the base's low 32 bits, so it
> +		 * is rebuilt rather than copied. Not modelled together with a
> +		 * delta, so skip if either side has one (sound, less precise).
> +		 */
> +		if (reg->subreg) {
> +			if (reg->add_const || known_reg->add_const)
> +				continue;
> +			reconstruct_zext32(reg, known_reg);
> +			if (e->is_reg)
> +				mark_reg_scratched(env, e->regno);
> +			else
> +				mark_stack_slot_scratched(env, e->spi);
> +			continue;
> +		}
> +		/*
> +		 * The reverse: known_reg knows only its low 32 bits, which say
> +		 * nothing about reg's high half.
> +		 */
> +		if (known_reg->subreg)
> +			continue;
>  		/*
>  		 * Skip mixed 32/64-bit links: the delta relationship doesn't
>  		 * hold across different ALU widths.

Can reconstruct_zext32() lose bounds that reg already learned on its own?

Looking at the code, reconstruct_zext32() does:

    *reg = *known_reg;
    reg->subreg = subreg;
    zext_32_to_64(reg);

This overwrites reg's state wholesale without intersecting it with what reg
already knew.

For pre-existing full-64-bit links, sync_linked_regs() propagates changes
in both directions, so the base is always at least as narrow as every
member and the overwrite can only narrow. But the new low-32 link skips
the member->base direction:

    if (known_reg->subreg)
        continue;

so the base can end up strictly wider than the ->subreg member.

Consider this sequence:

  call bpf_get_prandom_u32;    /* r0 -> 64-bit unknown */
  r6 = r0;                     /* r6 is 64-bit unknown */
  w7 = w6;                     /* r7->id = r6->id, r7->subreg = SUBREG_ZEXT */
  if w7 > 7 goto out;          /* fall-through: r7 u32 range [0,7]
                                * sync_linked_regs(known_reg=r7) skips r6 due
                                * to `if (known_reg->subreg) continue;`
                                * so r6 stays fully unknown */
  if r6 > 100 goto out;        /* fall-through: r6 narrows to [0,100]
                                * sync_linked_regs(known_reg=r6) reaches r7
                                * via reconstruct_zext32(r7, r6) which does
                                * *r7 = *r6 then zext_32_to_64() */

After the second jump, does r7 become [0,100] instead of staying [0,7]?

If so, a jump on an unrelated wide register would widen the bounds of a
narrow copy that the verifier had already proved tighter, on a
straight-line path. That would reject programs that narrowed the 32-bit
copy first and the wide original second, then relied on the copy's
narrower bound for a memory access or helper argument.

The changelog acknowledges the other precision loss this patch introduces
("forming one costs the source its delta ... That loses tracking a wide
source used to keep") and the follow-up selftest commit 36b84852a200 pins
it down with zext_mov_breaks_add_const_src. But neither the changelog nor
reconstruct_zext32()'s comment ("Copy the base to keep its precise low-32
tnum"), nor any of the 10 new tests in 36b84852a200, mentions or covers
this path. The only test that checks the member's fate is
zext_narrow_dst_keeps_base, which verifies the base is not narrowed but
does not check what happens to the member afterwards.

Would intersecting the rebuilt state with the member's existing one, or
narrowing the base's low half on the member->base direction instead of
skipping it, prevent the member from ever being wider than what it already
proved?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34506184282