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 <[email protected]>
> > ---
> > 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);
[...]