Re: [PATCH bpf-next v3 08/18] bpf: Check fixed-size mem args of helpers and kfuncs the same way

Amery Hung <[email protected]> Sun, 2 Aug 2026 15:10:13 -0700
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf
Message-ID <CAMB2axNktmWPs80p9Xw72Rr-Mexdbd6O5HfMXhgByVwscNW6sA@mail.gmail.com>
On Sat, Aug 1, 2026 at 1:17=E2=80=AFAM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issu=
e(s) to consider:
>
> New issues:
> - [High] Kfuncs taking uninitialized memory arguments via the `__uninit` =
annotation are falsely rejected when passed uninitialized stack slots.
>
> Pre-existing issues:
> - [Critical] The verifier fails to check if the size argument for `KF_ARG=
_PTR_TO_MEM_SIZE` kfuncs is a scalar, allowing BPF programs to pass kernel =
pointers as size arguments, leading to an out-of-bounds access.
> --
>
> --- Patch [8]: [PATCH bpf-next v3 08/18] bpf: Check fixed-size mem args o=
f helpers and kfuncs the same way ---
> commit bcee13ba62b78a9328164e2486f3c31a7fddb8a0
> Author: Amery Hung <[email protected]>
>
> bpf: Check fixed-size mem args of helpers and kfuncs the same way
>
> Fixed-size memory arguments went through two paths: helpers called
> check_helper_mem_access() directly, while kfuncs and global subprogs
> used check_mem_reg(). Route the helper MEM_FIXED_SIZE case through
> check_mem_reg() too so all three share the same check.
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index f61771e2a8b27..9e0c7f0a15c15 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
>
> [ ... ]
>
> > @@ -12379,7 +12385,7 @@ static int check_kfunc_args(struct bpf_verifier=
_env *env, struct bpf_call_arg_me
> >                                       ref_tname, PTR_ERR(resolve_ret));
> >                               return -EINVAL;
> >                       }
> > -                     ret =3D check_mem_reg(env, reg, argno, type_size,=
 meta);
> > +                     ret =3D check_mem_reg(env, reg, argno, type_size,=
 BPF_READ | BPF_WRITE, meta);
>
> [Severity: High]
> Does this change break BPF programs that pass uninitialized stack slots t=
o
> kfuncs using the __uninit annotation?
>
> Because check_mem_reg() is now unconditionally called with
> BPF_READ | BPF_WRITE here, it seems to mandate initialization for __unini=
t
> kfunc arguments as well, which should only require BPF_WRITE.

First of all, this is not a change introduced by this patch. kfunc
already calls check_helper_mem_access() with BPF_READ and then
BPF_WRITE.

This is also not an issue unless a a new unpreviledged kfunc that
takes __uninit arg is introduced. There are two kinds of existing
kfuncs with __uninit arg: bpf_dynptr_from_xxx() and
scx_bpf_cid_topo(). The former, while being unpreviledged kfuncs,
checks their __uninit dynptr argument through process_dynptr_func(),
which directly calls check_mem_access() to simulate byte-by-byte
write. The later is a previledged kfunc, which means
check_stack_range_initialized() will see env->allow_uninit_stack =3D
true and allow reading uninit stack slot on BPF_READ and then mark the
stack slot as STACK_MISC on BPF_WRITE.


>
> >                       if (ret < 0)
> >                               return ret;
> >                       break;
> >               case KF_ARG_PTR_TO_MEM_SIZE:
> >               {
> >                       struct bpf_reg_state *buff_reg =3D reg;
> >                       const struct btf_param *buff_arg =3D &args[i];
> >                       struct bpf_reg_state *size_reg =3D get_func_arg_r=
eg(caller, regs, i + 1);
> >                       const struct btf_param *size_arg =3D &args[i + 1]=
;
> >                       argno_t next_argno =3D argno_from_arg(i + 2);
> >
> >                       if (!bpf_register_is_null(buff_reg) || !is_kfunc_=
arg_nullable(meta->btf, buff_arg)) {
> >                               ret =3D check_mem_size_reg(env, buff_reg,=
 size_reg, argno, next_argno,
> >                                                        BPF_READ | BPF_W=
RITE, true, meta);
> >                               if (ret < 0) {
> [ ... ]
> >                       }
> >
> >                       /* Skip next '__sz' or '__szk' argument */
> >                       i++;
> >                       break;
>
> [Severity: Critical]
> This isn't a bug introduced by this patch, but does the verifier fail to
> enforce that the size argument is a scalar for KF_ARG_PTR_TO_MEM_SIZE?

Not a pre-existing bug. Currently get_kfunc_ptr_arg_type() does
mandate the register passed to __sz or __szk args to be scalar.

>
> The loop uses i++ to skip the next iteration, which completely bypasses t=
he
> btf_type_is_scalar validation for the size argument. check_mem_size_reg()
> also assumes the register is a SCALAR_VALUE without verifying it.
>
> If a BPF program passes a pointer register (like PTR_TO_STACK with a zero
> offset) as the size argument, reg_umax evaluates to 0, which passes the
> verifier's size checks. At runtime, the full kernel pointer address could=
 be
> used as the size, potentially allowing an out-of-bounds memory access.
>
> --
> Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260801074633.15=
[email protected]?part=3D8