Merge branch 'compare-stack-frames-in-exact-register-states'

Kumar Kartikeya Dwivedi says:

====================
Compare stack frames in exact register states

regs_exact() compares register values and their ID relationships, but it
does not compare frameno. regsafe() checks frameno for ordinary
PTR_TO_STACK comparisons, while its EXACT path returns through regs_exact()
before reaching that check. Infinite-loop detection can therefore mistake
pointers to the same offset in different stack frames for the same pointer
and reject a finite loop.

Move frameno into bpf_reg_state's type-specific metadata union so the
existing regs_exact() prefix comparison covers it. This avoids a separate
PTR_TO_STACK case and keeps the structure at 80 bytes. Adjust the
states_maybe_looping() comparison boundary for the new layout. Since
frameno now aliases other pointer metadata, bpf_func() returns NULL for
registers that are not stack pointers; the callers that look up the frame
before checking the register type dereference it only afterwards.

The selftest keeps a stack pointer live in a register across a loop
whose only change at the header is the pointer's frame number. On the
unfixed tree, the program is rejected with "infinite loop detected". With
the fix, it loads and returns the expected value.

Changelog:
----------
v3 -> v4
v3: https://lore.kernel.org/bpf/20260919004327.1403382-1-memxor@gmail.com

 * Return NULL from bpf_func() for non-stack registers, since frameno now
   aliases other pointer metadata and some callers look up the frame before
   checking the register type. (Sashiko)

v2 -> v3
v2: https://lore.kernel.org/bpf/20260918011313.3053497-1-memxor@gmail.com

 * Rebase on bpf/master.
 * Drop the redundant spilled-pointer test, since existing tests already
   cover the stacksafe() -> regsafe() path. (Eduard)
 * Place asm labels on their own line in the selftest. (Eduard)
 * Collect Acked-by and Tested-by tags.

v1 -> v2
v1: https://lore.kernel.org/bpf/20260914161340.3419141-1-memxor@gmail.com

 * Rebase on bpf/master.
 * Move frameno into the type-specific metadata union so regs_exact()'s
   existing prefix comparison covers it without growing bpf_reg_state.
====================

Link: https://patch.msgid.link/20260919014213.1840880-1-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

include/linux/bpf_verifier.h

@@ -45,6 +45,14 @@ struct bpf_reg_state {
 	union {
 		/* valid when type == PTR_TO_PACKET */
 		int range;
+		/*
+		 * Valid when type == PTR_TO_STACK. Inside the callee two registers
+		 * can be both PTR_TO_STACK like R1=fp-8 and R2=fp-8, but one of them
+		 * points to this function stack while another to the caller's stack.
+		 * To differentiate them 'frameno' is used which is an index in
+		 * bpf_verifier_state->frame[] array pointing to bpf_func_state.
+		 */
+		u8 frameno;
 
 		/*
 		 * For CONST_PTR_TO_MAP, PTR_TO_MAP_KEY, PTR_TO_MAP_VALUE and
@@ -155,14 +163,6 @@ struct bpf_reg_state {
 	 * during state comparisons.
 	 */
 	u32 map_uid;
-	/*
-	 * Inside the callee two registers can be both PTR_TO_STACK like
-	 * R1=fp-8 and R2=fp-8, but one of them points to this function stack
-	 * while another to the caller's stack. To differentiate them 'frameno'
-	 * is used which is an index in bpf_verifier_state->frame[] array
-	 * pointing to bpf_func_state.
-	 */
-	u8 frameno;
 	/* if (!precise && SCALAR_VALUE) min/max/tnum don't affect safety */
 	bool precise;
 };
@@ -1239,11 +1239,18 @@ static inline int bpf_get_spi(s32 off)
 	return (-off - 1) / BPF_REG_SIZE;
 }
 
+/*
+ * Return the function state a stack pointer register refers to. frameno
+ * shares storage with other pointer metadata, so return NULL for any
+ * other register type instead of indexing frame[] with aliased bytes.
+ */
 static inline struct bpf_func_state *bpf_func(struct bpf_verifier_env *env,
 					      const struct bpf_reg_state *reg)
 {
 	struct bpf_verifier_state *cur = env->cur_state;
 
+	if (reg->type != PTR_TO_STACK)
+		return NULL;
 	return cur->frame[reg->frameno];
 }
 

kernel/bpf/states.c

@@ -644,10 +644,7 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
 		return range_within(rold, rcur) &&
 		       tnum_in(rold->var_off, rcur->var_off);
 	case PTR_TO_STACK:
-		/* two stack pointers are equal only if they're pointing to
-		 * the same stack frame, since fp-8 in foo != fp-8 in bar
-		 */
-		return regs_exact(rold, rcur, idmap) && rold->frameno == rcur->frameno;
+		return regs_exact(rold, rcur, idmap);
 	case PTR_TO_ARENA:
 		return true;
 	case PTR_TO_INSN:
@@ -1126,7 +1123,7 @@ static bool states_maybe_looping(struct bpf_verifier_state *old,
 	fcur = cur->frame[fr];
 	for (i = 0; i < MAX_BPF_REG; i++)
 		if (memcmp(&fold->regs[i], &fcur->regs[i],
-			   offsetof(struct bpf_reg_state, frameno)))
+			   offsetof(struct bpf_reg_state, precise)))
 			return false;
 	return true;
 }

tools/testing/selftests/bpf/progs/verifier_loops1.c

@@ -303,4 +303,40 @@ __naked void maybe_exit_scc_bug1(void)
 	::: __clobber_all);
 }
 
+/*
+ * The loop reads zero from the caller's stack on its first iteration and
+ * one from the callee's stack on its second iteration. At the loop header,
+ * only the frame number of the pointer in r1 changes.
+ */
+static __naked __noinline __used
+void loop_stack_frames_reg(void)
+{
+	asm volatile (
+	"*(u64 *)(r10 - 8) = 1;"
+"1:"
+	"r0 = *(u64 *)(r1 + 0);"
+	"if r0 != 0 goto 2f;"
+	"r1 = r10;"
+	"r1 += -8;"
+	"goto 1b;"
+"2:"
+	"exit;"
+	::: __clobber_all);
+}
+
+SEC("xdp")
+__description("bounded loop changing stack frame in a register")
+__success __retval(1)
+__flag(BPF_F_TEST_STATE_FREQ)
+__naked void bounded_loop_stack_frames_reg(void)
+{
+	asm volatile (
+	"*(u64 *)(r10 - 8) = 0;"
+	"r1 = r10;"
+	"r1 += -8;"
+	"call loop_stack_frames_reg;"
+	"exit;"
+	::: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";