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

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

On 9/8/26 11:46 PM, sashiko-bot@kernel.org wrote:
> 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?

You are right. Will fix.

>