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

On 8/13/26 2:11 PM, [email protected] wrote:
>> 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. A
>> by-value struct or union return (of any size) was rejected at compile time
>> with:
>>
>>    error: aggregate returns are not supported
>>
>> and an __int128 return failed later in the backend with:
>>
>>    fatal error: error in backend: unable to allocate function return #1
>>
>> Both are resolved in LLVM 23, which lowers such returns into the R0:R2
>> register pair.
>>
>> This patch models that pair at calls to global and static BPF subprograms
>> and at kfunc calls: R2 is marked alongside R0 at the call, propagated out
>> of a callee at its exit, and held to the same scalar-only and no-stack-
>> pointer rules that R0 already is.
> This isn't a bug, but could the changelog be reworded in imperative mood
> to match Documentation/process/submitting-patches.rst? The other commits
> in this series follow that convention, so this one reads inconsistently.
>
>> @@ -9858,11 +9867,15 @@ 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];
>> -	if (r0->type == PTR_TO_STACK) {
>> +	nregs = bpf_ret_reg_pair(env, callee->subprogno) ? 2 : 1;
>> +	for (i = 0; i < nregs; i++) {
>> +		if (callee->regs[ret_regs[i]].type != PTR_TO_STACK)
>> +			continue;
>>   		/* technically it's ok to return caller's stack pointer
>>   		 * (or caller's caller's pointer) back to the caller,
>>   		 * since these pointers are valid. Only current stack
> The new R0:R2 return-register loop is placed before the
> 'if (callee->in_callback_fn)' branch, so it runs for callback frames as
> well. A callback follows bpf_callback_t (return value in R0 only) and R2
> holds an incoming argument: set_loop_callback_state() and
> set_user_ringbuf_callback_state() both do
> 'callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3]', i.e. the
> callback_ctx, which is PTR_TO_STACK for the common
> 'bpf_loop(n, cb, &local_var, 0)' idiom.
>
> bpf_compute_subprog_ret_regs() sets ret_reg_pair purely from the
> subprog's BTF return type and iterates every subprog, with no exclusion
> for subprogs used as callbacks. So a static subprog whose BTF declares a
> 9..16 byte return and which is passed to bpf_loop() or
> bpf_user_ringbuf_drain() gets ret_reg_pair = true, and at its BPF_EXIT
> the still-live callback_ctx in R2 is inspected as if it were the upper
> half of a return value.
>
> Consequence: the callback exit is rejected with 'cannot return stack
> pointer to the caller', a diagnostic that points at a register holding an
> argument rather than a return value, and it fires before the
> in_callback_fn R0 checks that would otherwise produce the accurate
> message. Impact is limited (any such program is already ill-typed with
> respect to bpf_callback_t, and compiler-generated code for a 16-byte
> return does write R2), so this is a misleading-rejection /
> modelling-scope defect rather than a memory-safety bug.
>
> Would it make sense to compute nregs after the in_callback_fn branch, or
> force nregs = 1 when callee->in_callback_fn, since a callback frame has
> no R0:R2 return convention?

The current callback function only returns one register. If the bpf
prog intends to return two registers, the upper R2 register is ignored.
The result may be incorrect. It is bpf prog users' problem.

>
>> @@ -9897,8 +9910,12 @@ 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]];
>>   	}
> prepare_func_exit() has three call sites, and only the real BPF_EXIT one
> (process_bpf_exit_full()) establishes the callee's return registers. The
> other two are synthetic exits that explicitly set up R0 but not R2,
> because R1-R5 were already reset to NOT_INIT above them:
>
> In check_helper_call(), BPF_FUNC_tail_call inside a subprog: the
> caller-saved reset loop sets R1-R5 to NOT_INIT, then only
> 'mark_reg_unknown(env, regs, BPF_REG_0)' runs before
> 'prepare_func_exit(env, &env->insn_idx)'.
>
> In check_ld_abs() hidden BPF_EXIT inside a subprog: reset loop, then only
> 'mark_reg_known_zero(env, regs, BPF_REG_0)' before prepare_func_exit().
>
> For a static subprog with ret_reg_pair set, the new copy loop therefore
> propagates a NOT_INIT R2 into caller->regs[BPF_REG_2] on those paths.
> This is fail-safe (no uninitialised value is treated as readable), but it
> means a caller that reads the upper half of the 16-byte return is
> rejected on that path with an opaque 'R2 !read_ok', so a subprog with a
>> 8 byte return that contains a bpf_tail_call() or LD_ABS is unverifiable,
> and only R0 gets the conservative unknown-scalar treatment the commit
> message describes.
>
> Would symmetric handling be to mark R2 unknown next to R0 at both sites
> (the tail-called program only defines R0, so R2 is genuinely undefined
> there), or to reject the combination with an explicit message?

Okay, this is related to tail call. The tail call prog expects to return
R0 only. If the bpf prog intends to return R0:R2 (similar to main program),
it is user's problem.

>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31740414277