Re: [PATCH bpf-next v3 15/18] bpf: Classify kfunc pointer arguments from BTF, resolve type against the register

On Sat, Aug 1, 2026 at 1:22 AM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Allowing PTR_TO_BTF_ID as a KF_ARG_PTR_TO_MEM argument is either broken or fundamentally unsafe due to unvalidated access flags.
> --
>
> --- Patch [15]: [PATCH bpf-next v3 15/18] bpf: Classify kfunc pointer arguments from BTF, resolve type against the register ---
> commit d5adf1f80d7ac5827091492c5099bcb0c05e15d1
> Author: Amery Hung <[email protected]>
>
> bpf: Classify kfunc pointer arguments from BTF, resolve type against the register
>
> This commit changes kfunc pointer argument classification to rely strictly
> on BTF rather than register state. It introduces support for passing a
> PTR_TO_BTF_ID as a fixed-size memory buffer when pointing to a scalar struct.
>
> >     The new design now accepts one previously rejected case: passing
> >     PTR_TO_BTF_ID to a pointer to scalar w/o a following __sz/__szk. The
> >     argument will be classified as KF_ARG_PTR_TO_MEM | MEM_FIXED_SIZE. The
> >     PTR_TO_BTF_ID register will go through check_mem_reg() ->
> >     check_helper_mem_access() -> check_ptr_to_btf_access(), and by default only
> >     read is allowed.
>
> [Severity: Medium]
> Will this newly accepted case always fail verification?
>
> While the commit message states that "by default only read is allowed,"
> the call to check_mem_reg() for KF_ARG_PTR_TO_MEM arguments
> unconditionally demands BPF_READ | BPF_WRITE access. This causes
> check_ptr_to_btf_access() to reject standard kernel pointers with -EACCES.

I see the problem in my wording. So it should be:

For a pointer to scalar arg, a kernel btf id will be rejected unless
explicitly granted by btf_struct_access(); a program allocated btf id
will be allowed.

>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 1ccf3b764c515..9045369ba5694 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -11405,37 +11414,37 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env,
> [ ... ]
> > -     /* This is the catch all argument type of register types supported by
> > -      * check_helper_mem_access. However, we only allow when argument type is
> > -      * pointer to scalar, or struct composed (recursively) of scalars. When
> > -      * arg_mem_size is true, the pointer can be void *.
> > +     /* A pointer to a struct without a size argument is classified as KF_ARG_PTR_TO_BTF_ID */
> > +     if (btf_type_is_struct(ref_t))
> > +             return KF_ARG_PTR_TO_BTF_ID;
> > +
> > +     /*
> > +      * Otherwise this is a fixed-size memory buffer supported by
> > +      * check_helper_mem_access(): a pointer to a scalar or a struct of
> > +      * scalars. The access size is derived from the pointed-to BTF type.
> >        */
> > -     if (!btf_type_is_scalar(ref_t) && !__btf_type_is_scalar_struct(env, meta->btf, ref_t, 0) &&
> > -         (arg_mem_size ? !btf_type_is_void(ref_t) : 1)) {
> > -             verbose(env, "%s pointer type %s %s must point to %sscalar, or struct with scalar\n",
> > -                     reg_arg_name(env, argno),
> > -                     btf_type_str(ref_t), ref_tname, arg_mem_size ? "void, " : "");
> > +     if (!btf_type_is_scalar(ref_t) &&
> > +         !__btf_type_is_scalar_struct(env, meta->btf, ref_t, 0)) {
> > +             verbose(env, "%s pointer type %s %s must point to scalar, or struct with scalar\n",
> > +                     reg_arg_name(env, argno), btf_type_str(ref_t), ref_tname);
> >               return -EINVAL;
> >       }
> > -     return arg_mem_size ? KF_ARG_PTR_TO_MEM : KF_ARG_PTR_TO_MEM | MEM_FIXED_SIZE;
> > +     return KF_ARG_PTR_TO_MEM | MEM_FIXED_SIZE;
> >  }
>
> [Severity: Medium]
> When get_kfunc_ptr_arg_type() returns KF_ARG_PTR_TO_MEM | MEM_FIXED_SIZE for
> a PTR_TO_BTF_ID, or when check_kfunc_args() falls through to KF_ARG_PTR_TO_MEM,
> it verifies the argument as a memory buffer:
>
> kernel/bpf/verifier.c:check_kfunc_args() {
>     ...
>                 case KF_ARG_PTR_TO_MEM:
>                         if (kf_arg_type & MEM_FIXED_SIZE) {
>                                 resolve_ret = btf_resolve_size(btf, ref_t, &type_size);
>                 ...
>                                 ret = check_mem_reg(env, reg, argno, type_size, BPF_READ | BPF_WRITE, meta);
>     ...
> }
>
> Because check_mem_reg() is passed BPF_READ | BPF_WRITE, it requires write
> access. When this goes through check_helper_mem_access() and reaches
> check_ptr_to_btf_access(), it will be rejected because atype != BPF_READ:
>
> kernel/bpf/verifier.c:check_ptr_to_btf_access() {
>     ...
>                 if (atype != BPF_READ && !type_is_ptr_alloc_obj(reg->type)) {
>                         verbose(env, "only read is supported\n");
>                         return -EACCES;
>                 }
>     ...
> }
>
> Could this unconditionally break the newly introduced feature for standard
> (non-allocated) kernel pointers? Also, if the BPF_READ | BPF_WRITE flag were
> simply changed to BPF_READ to pass verification, could it introduce a
> vulnerability since the verifier cannot guarantee that the underlying kfunc
> will not write to the buffer?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15