Re: [PATCH bpf-next v3 2/9] bpf: Assign reg->id when getting referenced kptr from ctx

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2026-04-22 at 15:45 -0700, Amery Hung wrote:
> On Wed, Apr 22, 2026 at 2:46 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Tue, 2026-04-21 at 15:10 -0700, Amery Hung wrote:
> > > Assign reg->id when getting referenced kptr from read program context
> > > to be consistent with R0 of KF_ACQUIRE kfunc. skb dynptr will track the
> > > referenced skb in qdisc programs using a new field reg->parent_id in
> > > a later patch.
> > > 
> > > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > > ---
> > >  kernel/bpf/verifier.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index 41e4ea41c72e..93003a2a96b0 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -6448,8 +6448,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> > >                       } else {
> > >                               mark_reg_known_zero(env, regs,
> > >                                                   value_regno);
> > > -                             if (type_may_be_null(info.reg_type))
> > > -                                     regs[value_regno].id = ++env->id_gen;
> > >                               /* A load of ctx field could have different
> > >                                * actual load size with the one encoded in the
> > >                                * insn. When the dst is PTR, it is for sure not
> > > @@ -6459,8 +6457,11 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> > >                               if (base_type(info.reg_type) == PTR_TO_BTF_ID) {
> > >                                       regs[value_regno].btf = info.btf;
> > >                                       regs[value_regno].btf_id = info.btf_id;
> > > +                                     regs[value_regno].id = info.ref_obj_id;
> > >                                       regs[value_regno].ref_obj_id = info.ref_obj_id;
> > >                               }
> > 
> > Hi Amery,
> > 
> > Could you please help me double-check my understanding of this change?
> > 
> > It adds a new behavior, if struct_ops method parameter is annotated
> > with __ref, then if a field is read from such struct_ops context
> > the destination register would now have it's .id == ref_obj_id
> > allocated for the parameter at the beginning of do_check().
> > E.g. for `rB = *(T *)(rA + off)`, if `rA` is a pointer to struct_ops
> > context and parameter corresponding to `off` has `__ref` annotation
> > the `rB.id` will be assigned, contrary to old behavior, when it remained 0.
> > 
> 
> Right

Ack, thank you.

> > This change in behavior, however, becomes visible only after the
> > changes to release_reference() later in the series.
> > There the `rB.id` is used to initiate `mark_reg_invalid(env, reg)` for
> > the `rB`.
> > 
> 
> I probably should mention it in the commit message. The purpose of the
> patch is to make the implementation consistent with the design:
> parent_id tracks the id of the parent objects.
> 
> Without this patch, it should still work since in this case ref_obj_id
> always equals to id. When release_reference() is called on a
> KF_RELEASE kfunc, the children objects will still be invalidated
> correctly.

bpf_qdisc_fail__invalid_dynptr_cross_frame.c:invalid_dynptr_cross_frame
fails if I comment out the line `regs[value_regno].id = info.ref_obj_id;`.
The added .id link is used to propagate invalidation triggered by
`free_skb(skb);` to  `hdr`.
The patch seem mandatory for the series.