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

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2026-08-10 at 17:09 -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 a callee's R2 could be considered dead and
> cleaned from checkpointed states, which would allow unsound state pruning.
> 
> Mark R2 as read at the BPF_EXIT of a subprogram that does return a register
> pair. bpf_compute_live_registers() walks the instructions in order and
> env->subprog_info[] is sorted by subprogram start, so the containing
> subprogram is tracked with a running index and its return convention is
> queried once per subprogram through bpf_ret_reg_pair().
> 
> Marking R2 at every exit instead would be simpler, but R2 would then stay
> live backwards across any call that is not followed by a write to R2, which
> is nearly every program, and would needlessly hurt state pruning.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

>  kernel/bpf/liveness.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index ef9a5a922887..451edd74fa6f 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c
> @@ -2062,10 +2062,15 @@ static inline u32 mask_widen(u32 m) { return m | (m << 16); }
>  static inline u16 mask_lo(u32 m) { return (u16)m; }
>  static inline u16 mask_hi(u32 m) { return (u16)(m >> 16); }
>  
> -/* Compute info->{use,def} fields for the instruction */
> +/*
> + * Compute info->{use,def} fields for the instruction. @ret_reg_pair tells
> + * whether the subprogram containing @insn returns its value in the R0:R2
> + * register pair, which matters for BPF_EXIT.
> + */

Nit: this comment change is unnecessary.

>  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,
> +				   bool ret_reg_pair)
>  {
>  	struct bpf_call_summary cs;
>  	const u8 class = BPF_CLASS(insn->code);
> @@ -2196,7 +2201,7 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env,
>  			break;
>  		case BPF_EXIT:
>  			def = 0;
> -			use = r0;
> +			use = ret_reg_pair ? (r0 | reg64_mask(BPF_REG_2)) : r0;

Nit: let's add an r2 variable.

>  			break;
>  		case BPF_CALL:
>  			def = ALL_CALLER_SAVED_REGS;

...