Re: [PATCH bpf-next 05/12] bpf: Account R2 of register-pair returns in live register analysis

[Yonghong Song <yonghong.song@linux.dev>]

On 7/9/26 2:07 PM, Eduard Zingerman wrote:
> On Wed, 2026-07-08 at 13:10 -0700, Yonghong Song wrote:
>> A BPF_EXIT of a subprogram returning a value larger than 8 bytes (a
>> struct/union or an __int128) reads R2 as well as R0, since the second half
>> of the return value is passed back in R2. compute_insn_live_regs() only
>> marked R0 used at exit, so the callee's R2 could be considered dead and
>> cleaned from checkpointed states, which would allow unsound state pruning.
>>
>> Mark R2 used at a BPF_EXIT when the enclosing subprogram returns via the
>> R0:R2 register pair. The call site already treats the caller-saved
>> registers R0-R5 as clobbered, so R2 as a result register is covered there.
>>
>> For example, for a subprogram returning an __int128, the live-registers
>> dump (log level 2) shows both R0 and R2 live before the callee's exit
>> (insn 5):
>>
>>    Live regs before insn:
>>          0: .12345.... (85) call pc+2
>>          1: ..2....... (bf) r0 = r2
>>          2: 0......... (95) exit
>>          3: .......... (b7) r0 = 1
>>          4: 0......... (b7) r2 = 2
>>          5: 0.2....... (95) exit
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   kernel/bpf/liveness.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
>> index 0aadfbae0acc..0977d32fcf01 100644
>> --- a/kernel/bpf/liveness.c
>> +++ b/kernel/bpf/liveness.c
>> @@ -2062,7 +2062,8 @@ struct insn_live_regs {
>>   /* Compute info->{use,def} fields for the instruction */
>>   static void compute_insn_live_regs(struct bpf_verifier_env *env,
>>   				   struct bpf_insn *insn,
>> -				   struct insn_live_regs *info)
>> +				   struct insn_live_regs *info,
>> +				   int subprog)
>>   {
>>   	struct bpf_call_summary cs;
>>   	u8 class = BPF_CLASS(insn->code);
>> @@ -2175,6 +2176,8 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env,
>>   		case BPF_EXIT:
>>   			def = 0;
>>   			use = r0;
>> +			if (bpf_ret_reg_pair(env, subprog))
>> +				use |= BIT(BPF_REG_2);
>>   			break;
>>   		case BPF_CALL:
>>   			def = ALL_CALLER_SAVED_REGS;
>> @@ -2237,8 +2240,12 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env)
>>   		goto out;
>>   	}
>>   
>> -	for (i = 0; i < insn_cnt; ++i)
>> -		compute_insn_live_regs(env, &insns[i], &state[i]);
>> +	for (i = 0, j = 0; i < insn_cnt; ++i) {
>> +		/* Advance to the subprog that contains instruction i. */
>> +		while (j + 1 < env->subprog_cnt && env->subprog_info[j + 1].start <= i)
>> +			j++;
> This loop is very in-elegant. The outer loop can be changed to iterate
> subprograms, with inner loop iterating instructions. Or just use
> bpf_find_subprog() when processing EXIT.

Ack. I prefer to have iterating subprogram in outer loop and iterating insns
in the inner loop.

>
>> +		compute_insn_live_regs(env, &insns[i], &state[i], j);
>> +	}
>>   
>>   	/* Forward pass: resolve stack access through FP-derived pointers */
>>   	err = bpf_compute_subprog_arg_access(env);