Re: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic

[Shung-Hsi Yu <shung-hsi.yu@suse.com>]

On Tue, Jul 21, 2026 at 05:12:14PM -0700, Eduard Zingerman wrote:
> On Tue, 2026-07-21 at 09:39 +0000, Yiyang Chen wrote:
> > When scalar += pointer is handled in adjust_ptr_min_max_vals(), the
> > destination register has to inherit the pointer register state from the
> > source pointer. Copying only selected fields is fragile because pointer
> > provenance is tracked by several bpf_reg_state fields.
> > 
> > For the commuted form, pass a temporary scalar offset register to the
> > common pointer arithmetic helper and copy the full pointer state before
> > applying pointer arithmetic. This preserves the frame number for
> > PTR_TO_STACK registers and keeps parent identity fields consistent.
> > 
> > Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")

This doesn't look right. Without bpf2bpf there is just one frame, hence
there isn't a bug at this point. I think the commit f4d7e40a5b71 ("bpf:
introduce function calls (verification)") the you mentioned in v1 should
be used here instead.

Ideally there should be a second tag for the missed parent_id
preservation issue for invalidating dynptr mentioned by Sashiko, too.
But it wasn't that obvious where that issue was introduced to me, and
f4d7e40a5b71 pre-dates dynptr anyway.

> > Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
> > ---
> >  kernel/bpf/verifier.c | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 52be0a118cce0..3c0af83db4672 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -13796,11 +13796,12 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> >  		return -EACCES;
> >  	}
> >  
> > -	/* In case of 'scalar += pointer', dst_reg inherits pointer type and id.
> > -	 * The id may be overwritten later if we create a new variable offset.
> > +	/* For 'scalar += pointer', dst_reg inherits the complete pointer
> > +	 * register state. Individual fields may be adjusted later by pointer
> > +	 * arithmetic.
> >  	 */
> > -	dst_reg->type = ptr_reg->type;
> > -	dst_reg->id = ptr_reg->id;
> > +	if (ptr_reg != dst_reg)
> > +		*dst_reg = *ptr_reg;
> 
> Lot's of tests are failing on the CI, please investigate.

I think one reason is that this overwrites r64 and var_off, hence in the
scalar += pointer case, we're adding the existing offset in ptr_reg to
itself, equivalent to:

    dst_reg->r64 = cnum64_add(ptr_reg->r64, ptr_reg->r64);
    dst_reg->var_off = tnum_add(ptr_reg->var_off, ptr_reg->var_off);

[...]