Re: [PATCH bpf-next v4 07/13] bpf: Add verifier support for 16-byte returns in R0:R2

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2026-08-10 at 17:09 -0700, Yonghong Song wrote:

...

> @@ -9810,10 +9827,14 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
>  	struct bpf_func_state *caller, *callee;
>  	struct bpf_reg_state *r0;
>  	bool in_callback_fn;
> +	u32 i, nregs;
>  	int err;
>  
>  	callee = state->frame[state->curframe];
>  	r0 = &callee->regs[BPF_REG_0];
> +	nregs = bpf_ret_reg_pair(env, callee->subprogno) ? 2 : 1;
> +	if (nregs > 1)
> +		env->prog->jit_required = 1;

Definitely let's move this to bpf_compute_subprog_ret_regs()
instead of setting it in multiple places.

>  	if (r0->type == PTR_TO_STACK) {
>  		/* technically it's ok to return caller's stack pointer
>  		 * (or caller's caller's pointer) back to the caller,
> @@ -9849,8 +9870,23 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
>  			return -EFAULT;
>  		}
>  	} else {
> -		/* return to the caller whatever r0 had in the callee */
> -		caller->regs[BPF_REG_0] = *r0;
> +		/*
> +		 * return to the caller whatever the callee had in the
> +		 * return register(s)
> +		 */
> +		for (i = 0; i < nregs; i++)
> +			caller->regs[ret_regs[i]] = callee->regs[ret_regs[i]];
> +
> +		/*
> +		 * R2 carries only the upper half of a register pair return
> +		 * value. A stack pointer must not escape the callee (see the
> +		 * R0 case above), but there is no need to reject the whole
> +		 * program for it: hand the caller an uninitialized R2 instead,
> +		 * so that only a caller actually using the returned pointer
> +		 * fails.
> +		 */
> +		if (nregs > 1 && caller->regs[BPF_REG_2].type == PTR_TO_STACK)
> +			bpf_mark_reg_not_init(env, &caller->regs[BPF_REG_2]);

Why special casing this? What if caller does not use r0,
should r0 be reset in such a case as well?
Let's handle both r0 and r2 in one place.

>  	}
>  
>  	/* for callbacks like bpf_loop or bpf_for_each_map_elem go back to callsite,

...

> @@ -16710,11 +16775,26 @@ static int check_global_subprog_return_code(struct bpf_verifier_env *env)
>  {
>  	struct bpf_func_state *cur_frame = cur_func(env);
>  	u32 subprog = cur_frame->subprogno;
> +	u32 i, nregs;
> +	int err;
>  
>  	if (subprog_returns_void(env, subprog))
>  		return 0;
>  
> -	return check_global_ret_scalar_reg(env, BPF_REG_0);
> +	/*
> +	 * An arena pointer is only a legitimate return value when it is the
> +	 * whole of it, that is when it is returned in R0 alone. Both halves of
> +	 * a register pair carry a piece of a >8 byte scalar, so an arena
> +	 * pointer in either of them is a leak.
> +	 */

Why forbidding returning two arena pointers?

> +	nregs = bpf_ret_reg_pair(env, subprog) ? 2 : 1;
> +	for (i = 0; i < nregs; i++) {
> +		err = check_global_ret_scalar_reg(env, ret_regs[i], nregs == 1);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
>  }
>  
>  /* Bitmask with 1s for all caller saved registers */
> @@ -17203,10 +17283,16 @@ static int process_bpf_exit_full(struct bpf_verifier_env *env,
>  	 */
>  	if (cur_frame->subprogno &&
>  	    !cur_frame->in_async_callback_fn &&
> -	    !cur_frame->in_exception_callback_fn)
> +	    !cur_frame->in_exception_callback_fn) {
>  		err = check_global_subprog_return_code(env);
> -	else
> +	} else {
> +		if (!cur_frame->subprogno && bpf_ret_reg_pair(env, 0)) {
> +			verbose(env,
> +				"return value larger than 8 bytes is not supported at program exit\n");
> +			return -EINVAL;
> +		}

Same as with callbacks, I don't see a reason to check this.
The purpose of the verifier is to avoid loading a program
that would accidentally bring down the kernel, this check
does not contribute towards this goal.

>  		err = check_return_code(env, BPF_REG_0, "R0");
> +	}
>  	if (err)
>  		return err;
>  	return PROCESS_BPF_EXIT;
> @@ -19366,6 +19452,22 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>  			return -EOPNOTSUPP;
>  		}
>  
> +		/*
> +		 * An extension replaces the target outright, so it has to match
> +		 * the target's return convention. Its own return value is capped
> +		 * at 8 bytes (a >8 byte program return is rejected at BPF_EXIT),
> +		 * so it can never fill the R0:R2 pair the target's callers read.
> +		 * This cannot be left to btf_check_type_match() above, which
> +		 * compares return types by btf_type->info only: an int carries no
> +		 * vlen, so a 16-byte __int128 and an 8-byte long compare equal.
> +		 */

Should the btf_check_type_match() be fixed?

> +		if (prog_extension && tgt_info->fmodel.ret_size > 8) {
> +			bpf_log(log,
> +				"Cannot replace function %s with a >8 byte return value\n",
> +				tname);
> +			return -EOPNOTSUPP;
> +		}
> +
>  		/*
>  		 * *.multi programs don't need an address during program
>  		 * verification, we just take the module ref if needed.