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

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]>
---
 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 0aadfbae0acc..68df6d8db4bd 100644
--- a/kernel/bpf/liveness.c
+++ b/kernel/bpf/liveness.c
@@ -2059,10 +2059,15 @@ struct insn_live_regs {
 /* Bitmask with 1s for all caller saved registers */
 #define ALL_CALLER_SAVED_REGS ((1u << CALLER_SAVED_REGS) - 1)
 
-/* 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.
+ */
 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;
 	u8 class = BPF_CLASS(insn->code);
@@ -2174,7 +2179,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 | BIT(BPF_REG_2)) : r0;
 			break;
 		case BPF_CALL:
 			def = ALL_CALLER_SAVED_REGS;
@@ -2209,8 +2214,8 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env)
 	struct bpf_insn *insns = env->prog->insnsi;
 	struct insn_live_regs *state;
 	int insn_cnt = env->prog->len;
-	int err = 0, i, j;
-	bool changed;
+	int err = 0, i, j, subprog, start, end;
+	bool changed, ret_reg_pair;
 
 	/* Use the following algorithm:
 	 * - define the following:
@@ -2237,8 +2242,14 @@ 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 (subprog = 0; subprog < env->subprog_cnt; subprog++) {
+		start = env->subprog_info[subprog].start;
+		end = env->subprog_info[subprog + 1].start;
+		ret_reg_pair = bpf_ret_reg_pair(env, subprog);
+
+		for (i = start; i < end; ++i)
+			compute_insn_live_regs(env, &insns[i], &state[i], ret_reg_pair);
+	}
 
 	/* Forward pass: resolve stack access through FP-derived pointers */
 	err = bpf_compute_subprog_arg_access(env);
-- 
2.53.0-Meta