Re: [PATCH bpf-next v7 06/10] bpf: Add verifier support for 16-byte returns in R0:R2

On 8/18/26 11:14 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added support for kfuncs returning structs is completely unreachable due to an overlooked check in btf_distill_func_proto, and hides a critical missing size limit.
> --
>
> --- Patch [6]: [PATCH bpf-next v7 06/10] bpf: Add verifier support for 16-byte returns in R0:R2 ---
> commit c9e349733faae615b87f8da3417558d2ede26348
> Author: Yonghong Song <[email protected]>
>
> bpf: Add verifier support for 16-byte returns in R0:R2
>
> LLVM 23 added support for returning a value in two registers for an
> __int128, or a struct/union whose size is greater than 8 but not more than
> 16 bytes. See LLVM patches [1] and [2].
>
> Before LLVM 23 the BPF backend could not return these values at all.
>
> [ ... ]
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 991782b402f20..d79f0c069795c 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> [ ... ]
>
>> @@ -13965,10 +13995,25 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>   	}
>>   
>>   	if (btf_type_is_scalar(t)) {
>> -		mark_reg_unknown(env, regs, BPF_REG_0);
>> +		mark_kfunc_ret_regs(env, regs, t->size);
>>   		if (meta.btf == btf_vmlinux && (meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock] ||
>>   		    meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave]))
>>   			__mark_reg_const_zero(env, &regs[BPF_REG_0]);
>> +	} else if (btf_type_is_struct(t)) {
>> +		/*
>> +		 * The returned struct comes back as raw register bits modeled
>> +		 * as an unknown scalar, so it must contain only scalars:
>> +		 * otherwise a pointer field would be laundered into a scalar
>> +		 * and escape provenance and reference tracking.
>> +		 */
> [Severity: Medium]
> Is this new struct return handling reachable?
>
> Looking at early program loading, btf_distill_func_proto() unconditionally
> rejects struct return types:
>
> kernel/bpf/btf.c:btf_distill_func_proto() {
>      ...
> 	if (ret < 0 || btf_type_is_struct(t)) {
> 		bpf_log(log,
> 			"The function %s return type %s is unsupported.\n",
> 			tname, btf_type_str(t));
> 		return -EINVAL;
> 	}
>      ...
> }
>
> As a result, wouldn't programs be rejected before check_kfunc_call() is
> executed?
>
> Additionally, if this code were reachable, does it lack a bounds check on
> t->size?
>
> Because kfuncs use the C ABI, returning a large struct requires a hidden
> 'sret' pointer. Without a size limit, returning a struct larger than 16 bytes
> might bypass the BPF return pair logic, as the BPF JIT would pass the
> attacker-controlled BPF R1 as this pointer.
>
> Could this potentially allow a kfunc to overwrite arbitrary kernel memory?

Patch 7 "bpf: Enable aggregate return types up to 16 bytes" will
have proper guard (e.g. return type must be <= 16 bytes). So we
should be fine here.

>
>> +		if (!__btf_type_is_scalar_struct(env, desc_btf, t, 0)) {
>> +			verbose(env,
>> +				"kernel function %s returns %s %s that is not composed of scalars\n",
>> +				func_name, btf_type_str(t),
>> +				btf_name_by_offset(desc_btf, t->name_off));
>> +			return -EINVAL;
>> +		}
>> +		mark_kfunc_ret_regs(env, regs, t->size);
>>   	} else if (btf_type_is_ptr(t)) {
>>   		ptr_type = btf_type_skip_modifiers(desc_btf, t->type, &ptr_type_id);
>>   		err = check_special_kfunc(env, &meta, regs, insn_aux, ptr_type, desc_btf);