[PATCH bpf 1/2] bpf: don't rewrite bpf_fastcall patterns entered by a jump

[Eduard Zingerman <eddyz87@gmail.com>]

mark_fastcall_pattern_for_call() must ensure that matched
"spill; call; fill" instruction series is not interrupted by a jump.
Otherwise the rewrite applied by bpf_remove_fastcall_spills_fills()
is not sound.

Record the instructions targeted by jumps in
insn_aux_data[*].jump_target when the CFG is built and use this flag
to stop growing a pattern at such an instruction. Jumps to the first
spill are fine.

Note that existing insn_aux_data[*].jmp_point field can't be reused,
as it marks subprogram return instructions.

Fixes: 5b5f51bff1b6 ("bpf: no_caller_saved_registers attribute for helper calls")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 include/linux/bpf_verifier.h | 12 ++++++++++++
 kernel/bpf/cfg.c             |  3 +++
 kernel/bpf/verifier.c        |  8 ++++++++
 3 files changed, 23 insertions(+)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 5fad59fdab0d..1339c2f028db 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -706,6 +706,8 @@ struct bpf_insn_aux_data {
 	 */
 	u32 calls_callback:1;
 	u32 indirect_target:1; /* if it is an indirect jump target */
+	/* true if some jump or call instruction targets this instruction */
+	u32 jump_target:1;
 	/*
 	 * CFG strongly connected component this instruction belongs to,
 	 * zero if it is a singleton SCC.
@@ -1142,6 +1144,16 @@ static inline void mark_jmp_point(struct bpf_verifier_env *env, int idx)
 	env->insn_aux_data[idx].jmp_point = true;
 }
 
+static inline void mark_jump_target(struct bpf_verifier_env *env, int idx)
+{
+	env->insn_aux_data[idx].jump_target = true;
+}
+
+static inline bool bpf_is_jump_target(struct bpf_verifier_env *env, int insn_idx)
+{
+	return env->insn_aux_data[insn_idx].jump_target;
+}
+
 static inline struct bpf_func_state *cur_func(struct bpf_verifier_env *env)
 {
 	struct bpf_verifier_state *cur = env->cur_state;
diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
index 0f13c13f4133..842c7d1eabcc 100644
--- a/kernel/bpf/cfg.c
+++ b/kernel/bpf/cfg.c
@@ -125,6 +125,7 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
 		/* mark branch target for state pruning */
 		mark_prune_point(env, w);
 		mark_jmp_point(env, w);
+		mark_jump_target(env, w);
 	}
 
 	if (insn_state[w] == 0) {
@@ -403,6 +404,7 @@ static int visit_gotox_insn(int t, struct bpf_verifier_env *env)
 		}
 
 		mark_jmp_point(env, w);
+		mark_jump_target(env, w);
 
 		/* EXPLORED || DISCOVERED */
 		if (insn_state[w])
@@ -564,6 +566,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
 
 		mark_prune_point(env, t + off + 1);
 		mark_jmp_point(env, t + off + 1);
+		mark_jump_target(env, t + off + 1);
 
 		return ret;
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e64035683795..fad774890953 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -17631,6 +17631,10 @@ bool bpf_get_call_summary(struct bpf_verifier_env *env, struct bpf_insn *call,
  *   r0 = *(u64 *)(r10 - 8);             r0 += r1;
  *   r0 += r1;                           exit;
  *   exit;
+ *
+ * Both uses of the marks assume that a pattern is entered at its first
+ * spill and thus executes as a unit, hence a pattern is not grown past
+ * an instruction targeted by a jump.
  */
 static void mark_fastcall_pattern_for_call(struct bpf_verifier_env *env,
 					   struct bpf_subprog_info *subprog,
@@ -17669,6 +17673,10 @@ static void mark_fastcall_pattern_for_call(struct bpf_verifier_env *env,
 	for (i = 1, off = lowest_off; i <= ARRAY_SIZE(caller_saved); ++i, off += BPF_REG_SIZE) {
 		if (insn_idx - i < 0 || insn_idx + i >= env->prog->len)
 			break;
+		/* stx/ldx/call must not be a jump targets, a jump to the first stx is fine */
+		if (bpf_is_jump_target(env, insn_idx - i + 1) ||
+		    bpf_is_jump_target(env, insn_idx + i))
+			break;
 		stx = &insns[insn_idx - i];
 		ldx = &insns[insn_idx + i];
 		/* must be a stack spill/fill pair */
-- 
2.55.0