Re: [PATCH bpf-next v1 0/6] bpf: Scope callback arguments to their frame

[Andrii Nakryiko]

On Mon, Sep 21, 2026 at 11:13 PM Ihor Solodrai <ihor.solodrai@xxxxxxxxx> wrote:
>
> On 2026-09-21 6:55 p.m., Alexei Starovoitov wrote:
> > On Mon, Sep 21, 2026 at 06:03 PM Ihor Solodrai <ihor.solodrai@xxxxxxxxx> wrote:
> >
> >> Neither has a local fix: both arguments point into a helper's own
> >> frame, with nothing longer-lived to anchor them to.
> >
> > It's the same problem as a pointer to callee's stack.
> > check_stack_write_fixed_off() deals with it like this:
> >    if (state != cur && reg->type == PTR_TO_STACK) {
> >            verbose(env, "cannot spill pointers to stack into stack frame of the caller\n");
> >            return -EINVAL;
> >    }
> > CONST_PTR_TO_DYNPTR wasn't spillable before 7.3, so nothing depends
> > on parking it in the caller's frame. Reject it there too ?
> > and then no need for REF_TYPE_FRAME complexity?
>
> If we focus on the nasty bpf_user_ringbuf_drain() bug specifically,
> then yes, check_stack_write_fixed_off() change patches it:
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d62c0f74cff5..f261423e9282 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3668,7 +3668,8 @@ static int check_stack_write_fixed_off(struct
> bpf_verifier_env *env,
>                          verbose(env, "invalid size of register spill\n");
>                          return -EACCES;
>                  }
> -               if (state != cur && reg->type == PTR_TO_STACK) {
> +               if (state != cur && (reg->type == PTR_TO_STACK ||
> +                                    reg->type == CONST_PTR_TO_DYNPTR)) {
>                          verbose(env, "cannot spill pointers to stack
> into stack frame of the caller\n");
>                          return -EINVAL;
>                  }
>
> However it doesn't cover some of the new test cases:
>    - user_ringbuf_callback_park_data_slice
>    - user_ringbuf_callback_park_kfunc_slice
>    - user_ringbuf_callback_park_clone
>    - user_ringbuf_callback_park_clone_then_slice
>
> (not counting the diag message diff)
>
> The original suggestion that came with the bug report was a
> cb_dynptr_id field in bpf_func_state set up in
> set_user_ringbuf_callback_state() and read in prepare_func_exit() to
> release it there.
>
> The cb_dynptr_id seemed way too specific, I didn't like it. So I've
> tried to figure out a feasible generalization of the problem, and came
> to "verifier can't track a lifetime of a ref tied to a frame", and
> then to this series.
>
> I think we need to decide whether the REF_TYPE_FRAME is a useful
> mechanism in principle, and whether it's sufficiently generic. It at
> least covers the cases in this series and more.
>
> For example AI also flagged for me parking the vma argument
> (PTR_TO_BTF_ID) of the bpf_find_vma() callback. It's low severity,
> which is why I excluded that from the series, but "reject a reg type"
> wouldn't work there AFAIU (we would break many legitimate programs).
>
> Opinions?
>

It does seem like we miss a reference representing single frame
lifetime which we can then use as a parent for all things that
shouldn't survive past the current frame. There is a separate fix
right now to invalidate dynptr created in subprog's frame. Seems like
that one should be expressible the same way: local dynptr's parent is
"frame lifetime" reference.

We can even relax this "cannot spill pointers to stack into stack
frame of the caller" error, potentially, by just turning that
PTR_TO_STACK into SCALAR at exit from current frame and let program
verification go, no harm.

So for me it seems like a necessary and useful addition.