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

Yonghong Song <[email protected]>
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
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 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.
+ */
 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;
 			break;
 		case BPF_CALL:
 			def = ALL_CALLER_SAVED_REGS;
@@ -2233,8 +2238,8 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env)
 	struct insn_live_regs *state;
 	int insn_cnt = env->prog->len;
 	u64 pos, insn_pos;
-	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:
@@ -2261,8 +2266,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
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.