Re: [PATCH bpf-next v2 3/5] bpf: Allow subprogs to return arena pointers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Global subprograms returning arena pointers have their return value unconditionally marked as a scalar at the call site.
- [Medium] The verifier bypasses BTF type enforcement for global subprogram return values.
- [Low] The error message logged when a global function has an invalid return type is outdated and confusing.
--

commit 0192bc82392a7d1bde4b6ab66bef8ece2f90c7cf
Author: Emil Tsalapatis <emil@etsalapatis.com>

bpf: Allow subprogs to return arena pointers

This patch expands the verifier to permit returning an arena pointer to the
caller from a subprogram. The main subprog is still not allowed to return
an arena pointer.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index c6a930aca67ec..7a8101879f84d 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c

[ ... ]

> @@ -7963,17 +7993,15 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
[ ... ]
> -	if (!btf_type_is_void(t) && !btf_type_is_int(t) && !btf_is_any_enum(t)) {
> -		if (!is_global)
> -			return -EINVAL;
> -		bpf_log(log,
> -			"Global function %s() return value not void or scalar. "
> -			"Only those are supported.\n",
> -			tname);
> +
> +	err = btf_validate_return_type(env, btf, t, subprog);
> +	if (err) {
> +		if (is_global) {
> +			bpf_log(log,
> +				"Global function %s() return value not void or scalar. "
> +				"Only those are supported.\n",
> +				tname);
> +		}
>  		return -EINVAL;
>  	}

[Severity: Low]
Since arena pointers are now supported, should this error message be
updated to avoid confusing users? It still states that only void or
scalars are supported.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index c8d980fdd7099..64d7954e93ea7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9375,7 +9375,7 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  		clear_caller_saved_regs(env, caller->regs);
>  		invalidate_outgoing_stack_args(env, cur_func(env));
>  
> -		/* All non-void global functions return a 64-bit SCALAR_VALUE. */
> +		/* All non-void global functions return a 64-bit SCALAR_VALUE or PTR_TO_ARENA. */
>  		if (!subprog_returns_void(env, subprog)) {
>  			mark_reg_unknown(env, caller->regs, BPF_REG_0);
>  			caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
>  		}

[Severity: High]
Does this unconditionally mark the caller's R0 register as a SCALAR_VALUE?

When returning from a global subprogram, mark_reg_unknown() clears the
register state. If the subprogram actually returned an arena pointer,
it seems the caller will receive an untracked scalar instead. Any attempt
by the caller to dereference it would then fail verification.

Should we inspect the subprogram's BTF return type here to dynamically
set caller->regs[BPF_REG_0].type = PTR_TO_ARENA when appropriate?

> @@ -16645,6 +16645,10 @@ static int check_global_subprog_return_code(struct bpf_verifier_env *env)
>  	if (err)
>  		return err;
>  
> +	/* Pointers to arena are safe to pass between subprograms. */
> +	if (is_arena_reg(env, BPF_REG_0))
> +		return 0;
> +
>  	if (is_pointer_value(env, BPF_REG_0)) {
>  		verbose(env, "R%d leaks addr as return value\n", BPF_REG_0);
>  		return -EACCES;

[Severity: Medium]
Does this bypass BTF type enforcement for the return value?

By just checking if R0 is an arena register and returning 0, a global
subprogram declared in BTF as returning an int could successfully return
a PTR_TO_ARENA. Conversely, a subprogram declared as returning an arena
pointer could successfully return a SCALAR_VALUE, since the fallback scalar
check later in the function would allow it.

Should we verify that the actual register type matches the subprogram's
BTF return type signature?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530002259.4505-1-emil@etsalapatis.com?part=3