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