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

Eduard Zingerman <[email protected]>
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
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 <[email protected]>
> ---

Acked-by: Eduard Zingerman <[email protected]>

>  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;

...
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.