Re: [PATCH bpf-next 03/12] bpf: Support by-value struct arguments up to 16 bytes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Out-of-bounds array write in `btf_prepare_func_args` when an argument passed by value requires multiple slots, overflowing the `sub->args` array.
--

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

bpf: Support by-value struct arguments up to 16 bytes

This commit adds support for passing struct arguments up to 16 bytes by value
to global BPF functions. The arguments are split into multiple register slots
based on their size.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 22828b489b778..dbd64cee1b27a 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8103,6 +8103,14 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  	for (i = 0, slots_used = 0; i < nargs; i++) {
>  		u32 tags = 0;
>  
> +		if (slots_used >= MAX_BPF_FUNC_ARGS) {
> +			if (!is_global)
> +				return -EINVAL;
> +			bpf_log(log, "Arguments of %s() need more than %d argument slots\n",
> +				tname, MAX_BPF_FUNC_ARGS);
> +			return -EINVAL;
> +		}
> +

[ ... ]

> @@ -8231,6 +8239,31 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  			sub->args[slots_used++].arg_type = ARG_ANYTHING;
>  			continue;
>  		}
> +		if (btf_type_is_struct(t)) {
> +			u32 nslots;
> +
> +			if (!t->size || t->size > 2 * BPF_REG_SIZE) {
> +				if (!is_global)
> +					return -EINVAL;
> +				bpf_log(log,
> +					"Arg#%d type %s in %s() has size %u, only 1 to %d bytes "
> +					"can be passed by value\n",
> +					i, btf_type_str(t), tname, t->size, 2 * BPF_REG_SIZE);
> +				return -EINVAL;
> +			}
> +			if (!btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR)) {
> +				if (!is_global)
> +					return -EINVAL;
> +				bpf_log(log, "Arg#%d type %s in %s() is not composed of scalars\n",
> +					i, btf_type_str(t), tname);
> +				return -EINVAL;
> +			}
> +
> +			nslots = (t->size + BPF_REG_SIZE - 1) / BPF_REG_SIZE;
> +			while (nslots--)
> +				sub->args[slots_used++].arg_type = ARG_ANYTHING;

[Severity: High]
Can this loop write out-of-bounds of the sub->args[] array?

The bounds check for slots_used is only performed at the beginning of the
outer for-loop in btf_prepare_func_args(). If a function argument is evaluated
when slots_used is exactly MAX_BPF_FUNC_ARGS - 1, and this argument is a
16-byte struct requiring 2 slots (nslots = 2), the inner while loop will
increment slots_used twice.

This will cause an out-of-bounds write when setting arg_type for the second
slot.

Since struct bpf_subprog_info defines args as having exactly
MAX_BPF_FUNC_ARGS elements, writing past the end will corrupt adjacent
memory fields like stack_arg_cnt.

Should the code verify if slots_used + nslots > MAX_BPF_FUNC_ARGS before
entering this loop?

> +			continue;
> +		}

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