Re: [PATCH bpf-next 06/12] bpf: Add a JIT helper for the outgoing stack of kfunc calls

[Yonghong Song <yonghong.song@linux.dev>]

On 9/3/26 10:25 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] An out-of-bounds write occurs in `btf_prepare_func_args` when processing a BPF subprogram with arguments that consume 11 slots followed by a final 16-byte struct argument.
> --
>
> commit d1c6556d5925344f81e73a6b534000285b7cee65
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Support by-value struct arguments up to 16 bytes
>
> This commit adds support for global functions taking a struct or union by
> value up to 16 bytes. It handles structs passed in consecutive registers
> or split across registers and the stack, computing the necessary argument
> slots. The stack argument count is then recomputed based on the slots
> consumed to ensure proper JIT support checks.
>
>> 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;
>> +		}
>> +
>>   		err = btf_scan_decl_tags(env, btf, fn_t, i, is_global, &tags);
>>   		if (err)
>>   			return err;
> [ ... ]
>
>> @@ -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]
> Does this loop write beyond the end of the sub->args[] array?
>
> If a non-global BPF subprogram has arguments consuming exactly 11 slots,
> slots_used >= MAX_BPF_FUNC_ARGS evaluates to false (11 >= 12) at the start
> of the loop, allowing it to proceed.
>
> When a final 16-byte struct argument follows, nslots is set to 2. This
> while loop then writes to sub->args[11] and subsequently out-of-bounds to
> sub->args[12].
>
> Could this memory corruption overwrite the adjacent stack_arg_cnt and
> max_out_stack_arg_cnt fields in struct bpf_subprog_info?

The same issue similar to "bpf: Support by-value struct arguments up to 16 bytes"
and "bpf: Support __int128 as a by-value function argument". Will fix.

>
> On big-endian architectures, this might artificially inflate
> max_out_stack_arg_cnt to 9 (the value of ARG_ANYTHING), causing the
> verifier to subsequently reject valid BPF programs in
> check_outgoing_stack_args(). On little-endian architectures it would
> silently zero the field.
>
>> +			continue;
>> +		}
>>   		if (!is_global)
>>   			return -EINVAL;