Re: [PATCH bpf-next v2 06/12] bpf: Recognize by-value struct and __int128 kfunc arguments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] bpf_kfunc_stack_access_bytes incorrectly maps stack slots to BPF_REG_6 through BPF_REG_9 (or higher), causing an out-of-bounds read and bypassing uninitialized memory checks.
--

commit 7f4b25d0dad6ba6fed374b3111fcc89d67434d87
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Recognize by-value struct and __int128 kfunc arguments

This commit modifies the BPF verifier to support kfuncs taking structs or
int128 by value. It updates argument tracking to walk slots rather than just
registers, rejecting multi-slot arguments until JIT support is fully added.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index cf526f28f3e5b..94c359351bb69 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -13765,23 +13850,32 @@ s64 bpf_kfunc_stack_access_bytes(struct bpf_verifier_env *env, struct bpf_insn *
>  	btf = meta.btf;
>  	args = btf_params(meta.func_proto);
>  	nargs = btf_type_vlen(meta.func_proto);
> -	if (arg >= nargs)
> +
> +	/*
> +	 * @arg is an argument slot and a 16-byte parameter takes two of them,
> +	 * so walk the parameters to find the one that starts at this slot. A
> +	 * slot holding the upper eightbyte of such a parameter belongs to no
> +	 * pointer, and neither does a slot past the last parameter.
> +	 */
> +	for (i = 0, slot = 0; i < nargs && slot < arg; i++)
> +		slot += kfunc_arg_slots(btf_type_skip_modifiers(btf, args[i].type, NULL));
> +	if (i >= nargs || slot != arg)
>  		return 0;
>  
> -	t = btf_type_skip_modifiers(btf, args[arg].type, NULL);
> +	t = btf_type_skip_modifiers(btf, args[i].type, NULL);
>  	if (!btf_type_is_ptr(t))
>  		return 0;
>  
>  	/* dynptr: fixed 16-byte on-stack representation */
> -	if (is_kfunc_arg_dynptr(btf, &args[arg])) {
> +	if (is_kfunc_arg_dynptr(btf, &args[i])) {
>  		size = BPF_DYNPTR_SIZE;
>  		goto out;
>  	}
>  
>  	/* ptr + __sz/__szk pair: size is in the next register */
> -	if (arg + 1 < nargs &&
> -	    (btf_param_match_suffix(btf, &args[arg + 1], "__sz") ||
> -	     btf_param_match_suffix(btf, &args[arg + 1], "__szk"))) {
> +	if (i + 1 < nargs &&
> +	    (btf_param_match_suffix(btf, &args[i + 1], "__sz") ||
> +	     btf_param_match_suffix(btf, &args[i + 1], "__szk"))) {
>  		int size_reg = BPF_REG_1 + arg + 1;
>  
>  		if (aux->const_reg_mask & BIT(size_reg)) {
>  			size = (s64)aux->const_reg_vals[size_reg];

[Severity: High]
This is a pre-existing issue, but does this code lead to an out-of-bounds
read or bypass uninitialized memory checks when the pointer argument is
passed on the stack?

The size_reg calculation inherently assumes the size argument is always passed
in a register. If the pointer argument slot is 4 or higher (which might be
more common now that large by-value structs consume multiple slots), the size
argument lands on the stack.

In that scenario, size_reg maps to BPF_REG_6 or higher, which are
callee-saved registers. A BPF program could place a fake constant in one of
these registers to trick the verifier into thinking 0 bytes are read, while
the runtime stack argument is non-zero. This could bypass stack
initialization checks and potentially leak uninitialized kernel memory.

Furthermore, if the pointer argument slot is 8 or higher, size_reg becomes
greater than or equal to 10. Checking BIT(size_reg) and reading from
aux->const_reg_vals[size_reg] would result in an out-of-bounds array read,
since const_reg_vals is sized for 10 registers. Should this logic be updated
to correctly query the BPF stack when the size argument is passed there?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909062522.4001896-1-yonghong.song@linux.dev?part=6