Re: [PATCH bpf-next v3 1/9] bpf: Unify dynptr handling in the verifier

Andrii Nakryiko
2026-04-24T16:23:02Z

On Thu, Apr 23, 2026 at 10:34 PM Amery Hung <ameryhung@gmail.com> wrote:
>
> On Thu, Apr 23, 2026 at 5:04 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Apr 21, 2026 at 3:10 PM Amery Hung <ameryhung@gmail.com> wrote:
> > >
> > > Simplify dynptr checking for helper and kfunc by unifying it. Remember
> > > the initialized dynptr (i.e.,g !(arg_type |= MEM_UNINIT)) pass to a
> > > dynptr kfunc during process_dynptr_func() so that we can easily
> > > retrieve the information for verification later. By saving it in
> > > meta->dynptr, there is no need to call dynptr helpers such as
> > > dynptr_id(), dynptr_ref_obj_id() and dynptr_type() in check_func_arg().
> > >
> > > Remove and open code the helpers in process_dynptr_func() when
> > > saving id, ref_obj_id, and type. It is okay to drop spi < 0 check as
> > > is_dynptr_reg_valid_init() has made sure the dynptr is valid.
> > >
> > > Besides, since dynptr ref_obj_id information is now pass around in
> > > meta->bpf_dynptr_desc, drop the check in helper_multiple_ref_obj_use.
> > >
> > > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> > > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > > ---
> > > include/linux/bpf_verifier.h | 12 ++-
> > > kernel/bpf/verifier.c | 178 +++++++----------------------------
> > > 2 files changed, 41 insertions(+), 149 deletions(-)
> > >
> >
> > [...]
> >
> > > @@ -7433,7 +7426,8 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno,
> > > * and checked dynamically during runtime.
> > > */
> > > static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn_idx,
> > > - enum bpf_arg_type arg_type, int clone_ref_obj_id)
> > > + enum bpf_arg_type arg_type, int clone_ref_obj_id,
> > > + struct bpf_dynptr_desc *dynptr)
> > > {
> > > struct bpf_reg_state *reg = reg_state(env, regno);
> > > int err;
> > > @@ -7499,6 +7493,20 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn
> > > }
> > >
> > > err = mark_dynptr_read(env, reg);
> >
> > if mark_dynptr_read() fails, should we exit instead of proceeding to
> > fill out dynptr info?.. even if mark_dynptr_read() cannot fail because
> > of is_dynptr_reg_valid_init(), it still looks wrong and error-prone
>
> I think I will change mark_dynptr_read() to receive spi from the
> caller and make the function return void (It is technically nofail
> after liveness tracking code is removed). Then, move dynptr_get_spi()
> before it and don't error out. A comment seems to be enough.

yep, makes sense

>
> /* Cannot fail as we already make sure reg is valid dynptr */
> spi = dynptr_get_spi(env, reg);
>
> mark_dynptr_read(env, reg, spi);
>
> >
> > > +
> > > + if (dynptr) {
> > > + struct bpf_func_state *state = bpf_func(env, reg);
> > > + int spi;
> > > +
> > > + if (reg->type != CONST_PTR_TO_DYNPTR) {
> > > + spi = dynptr_get_spi(env, reg);
>
> ^ move this up
>
> > > + reg = &state->stack[spi].spilled_ptr;
> > > + }
> > > +
> > > + dynptr->id = reg->id;
> > > + dynptr->type = reg->dynptr.type;
> > > + dynptr->ref_obj_id = reg->ref_obj_id;
> > > + }
> > > }
> > > return err;
> > > }
> >
> > …