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

[sashiko-bot@kernel.org]

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 <yonghong.song@linux.dev>

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?

> +		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);

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