mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Woojin Ji <random6.xyz@gmail.com>
To: Alexei Starovoitov <ast@kernel.org>,
	 Daniel Borkmann <daniel@iogearbox.net>,
	 John Fastabend <john.fastabend@gmail.com>,
	 Andrii Nakryiko <andrii@kernel.org>,
	 Martin KaFai Lau <martin.lau@linux.dev>,
	 Eduard Zingerman <eddyz87@gmail.com>,
	 Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Song Liu <song@kernel.org>,
	 Yonghong Song <yonghong.song@linux.dev>,
	Jiri Olsa <jolsa@kernel.org>,  Shuah Khan <shuah@kernel.org>
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	 linux-kselftest@vger.kernel.org,
	Woojin Ji <random6.xyz@gmail.com>
Subject: [PATCH bpf] bpf: Track spilled zero scalars for var-off stack reads
Date: Thu, 11 Jun 2026 19:11:24 +0900	[thread overview]
Message-ID: <20260611-bpf-stack-var-off-zero-v1-v1-1-0ec407376147@gmail.com> (raw)

mark_reg_stack_read() currently treats a variable-offset stack read as
known-zero only when every byte in the read range has STACK_ZERO type.
This loses precision for stack bytes that are represented as STACK_SPILL
but come from a spilled scalar const zero.

Fixed-offset stack reads already preserve such partial reads from a
spilled scalar zero as const zero. Variable-offset stack writes also keep
a spilled scalar zero intact when a zero write overlaps it. The read side
is therefore inconsistent and can reject otherwise valid programs: a byte
read from a spilled zero becomes an unknown u8 and may then make a
stack access through that value appear out of bounds.

Treat STACK_SPILL bytes backed by a spilled scalar const zero as zero
bytes in mark_reg_stack_read(). When such a spilled scalar is used to
prove the destination register is const zero, mark every contributing
source stack slot precise before state pruning can use an explored
zero-spill path for a later non-zero spill path.

This has to be done eagerly for variable-offset loads. Fixed-offset stack
fills can record one source stack slot in the jump history and propagate
destination precision back to that slot later, but a variable-offset load
may source bytes from multiple stack slots. Seed precision backtracking
with every zero-spill slot that contributes to the const-zero
classification instead.

Add verifier selftests for both sides: accepted programs that read a byte
through a variable stack offset from spilled zero scalars, including a
multi-slot range, and a rejected program that would be accepted unsafely
by a naive implementation that promotes spilled zero bytes to const zero
without tracking the source spilled slot precisely. Use
BPF_F_TEST_STATE_FREQ for the rejected test so it does not depend on the
current verifier checkpoint heuristic thresholds.

Tested:
  ./test_progs -t verifier_var_off

Assisted-by: opencode:gpt-5.5
Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
---
 kernel/bpf/verifier.c                              | 52 ++++++++++---
 .../testing/selftests/bpf/progs/verifier_var_off.c | 88 ++++++++++++++++++++++
 2 files changed, 130 insertions(+), 10 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7fb88e1cd..c4b89fbd9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4058,14 +4058,21 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env,
  * SCALAR. This function does not deal with register filling; the caller must
  * ensure that all spilled registers in the stack range have been marked as
  * read.
+ *
+ * If the const-zero classification depends on spilled scalar zeroes, mark the
+ * contributing stack slots precise so pruning cannot reuse a zero-spill state
+ * for a later path containing a different spilled scalar.
+ *
+ * Returns an error if precision backtracking fails.
  */
-static void mark_reg_stack_read(struct bpf_verifier_env *env,
-				/* func where src register points to */
-				struct bpf_func_state *ptr_state,
-				int min_off, int max_off, int dst_regno)
+static int mark_reg_stack_read(struct bpf_verifier_env *env,
+			       /* func where src register points to */
+			       struct bpf_func_state *ptr_state,
+			       int min_off, int max_off, int dst_regno)
 {
 	struct bpf_verifier_state *vstate = env->cur_state;
 	struct bpf_func_state *state = vstate->frame[vstate->curframe];
+	u64 zero_spill_mask = 0;
 	int i, slot, spi;
 	u8 *stype;
 	int zeros = 0;
@@ -4075,19 +4082,38 @@ static void mark_reg_stack_read(struct bpf_verifier_env *env,
 		spi = slot / BPF_REG_SIZE;
 		mark_stack_slot_scratched(env, spi);
 		stype = ptr_state->stack[spi].slot_type;
-		if (stype[slot % BPF_REG_SIZE] != STACK_ZERO)
-			break;
-		zeros++;
+		if (stype[slot % BPF_REG_SIZE] == STACK_ZERO) {
+			zeros++;
+			continue;
+		}
+		if (stype[slot % BPF_REG_SIZE] == STACK_SPILL &&
+		    bpf_is_spilled_scalar_reg(&ptr_state->stack[spi]) &&
+		    tnum_is_const(ptr_state->stack[spi].spilled_ptr.var_off) &&
+		    ptr_state->stack[spi].spilled_ptr.var_off.value == 0) {
+			zero_spill_mask |= 1ull << spi;
+			zeros++;
+			continue;
+		}
+		break;
 	}
 	if (zeros == max_off - min_off) {
 		/* Any access_size read into register is zero extended,
 		 * so the whole register == const_zero.
 		 */
 		__mark_reg_const_zero(env, &state->regs[dst_regno]);
+		if (zero_spill_mask) {
+			for (spi = 0; spi < MAX_BPF_STACK / BPF_REG_SIZE; spi++) {
+				if (zero_spill_mask & (1ull << spi))
+					bpf_bt_set_frame_slot(&env->bt, ptr_state->frameno, spi);
+			}
+			return mark_chain_precision_batch(env, env->cur_state);
+		}
 	} else {
 		/* have read misc data from the stack */
 		mark_reg_unknown(env, state->regs, dst_regno);
 	}
+
+	return 0;
 }
 
 /* Read the stack at 'off' and put the results into the register indicated by
@@ -4109,6 +4135,7 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
 	int i, slot = -off - 1, spi = slot / BPF_REG_SIZE;
 	struct bpf_reg_state *reg;
 	u8 *stype, type;
+	int err;
 	int insn_flags = insn_stack_access_flags(reg_state->frameno, spi);
 
 	stype = reg_state->stack[spi].slot_type;
@@ -4235,8 +4262,11 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
 			}
 			return -EACCES;
 		}
-		if (dst_regno >= 0)
-			mark_reg_stack_read(env, reg_state, off, off + size, dst_regno);
+		if (dst_regno >= 0) {
+			err = mark_reg_stack_read(env, reg_state, off, off + size, dst_regno);
+			if (err)
+				return err;
+		}
 		insn_flags = 0; /* we are not restoring spilled register */
 	}
 	if (insn_flags)
@@ -4291,7 +4321,9 @@ static int check_stack_read_var_off(struct bpf_verifier_env *env,
 
 	min_off = reg->smin_value + off;
 	max_off = reg->smax_value + off;
-	mark_reg_stack_read(env, ptr_state, min_off, max_off + size, dst_regno);
+	err = mark_reg_stack_read(env, ptr_state, min_off, max_off + size, dst_regno);
+	if (err)
+		return err;
 	check_fastcall_stack_contract(env, ptr_state, env->insn_idx, min_off);
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/verifier_var_off.c b/tools/testing/selftests/bpf/progs/verifier_var_off.c
index f345466bc..2d4878270 100644
--- a/tools/testing/selftests/bpf/progs/verifier_var_off.c
+++ b/tools/testing/selftests/bpf/progs/verifier_var_off.c
@@ -59,6 +59,94 @@ __naked void stack_read_priv_vs_unpriv(void)
 "	::: __clobber_all);
 }
 
+SEC("cgroup/skb")
+__description("variable-offset stack read preserves spilled zero")
+__success
+__failure_unpriv __msg_unpriv("R2 variable stack access prohibited for !root")
+__retval(0)
+__naked void stack_read_var_off_preserves_spilled_zero(void)
+{
+	asm volatile (
+	"r0 = 0; "
+	" *(u64 *)(r10 - 8) = r0; "
+	"r2 = *(u32 *)(r1 + 0); "
+	"r2 &= 7; "
+	"r2 -= 8; "
+	"r2 += r10; "
+	"r3 = *(u8 *)(r2 + 0); "
+	"r1 = r10; "
+	"r1 += -1; "
+	"r1 += r3; "
+	" *(u8 *)(r1 + 0) = r3; "
+	"r0 = 0; "
+	"exit; "
+	::
+	: __clobber_all);
+}
+
+SEC("cgroup/skb")
+__description("variable-offset stack read preserves spilled zero across slots")
+__success
+__failure_unpriv __msg_unpriv("R2 variable stack access prohibited for !root")
+__retval(0)
+__naked void stack_read_var_off_preserves_spilled_zero_across_slots(void)
+{
+	asm volatile (
+	"r0 = 0; "
+	" *(u64 *)(r10 - 8) = r0; "
+	" *(u64 *)(r10 - 16) = r0; "
+	"r2 = *(u32 *)(r1 + 0); "
+	"r2 &= 15; "
+	"r2 -= 16; "
+	"r2 += r10; "
+	"r3 = *(u8 *)(r2 + 0); "
+	"r1 = r10; "
+	"r1 += -1; "
+	"r1 += r3; "
+	" *(u8 *)(r1 + 0) = r3; "
+	"r0 = 0; "
+	"exit; "
+	::
+	: __clobber_all);
+}
+
+SEC("cgroup/skb")
+__description("variable-offset stack read tracks spilled zero precisely")
+__failure
+__flag(BPF_F_TEST_STATE_FREQ)
+__msg("invalid variable-offset write to stack R1")
+__failure_unpriv __msg_unpriv("R2 variable stack access prohibited for !root")
+__naked void stack_read_var_off_tracks_spilled_zero_precisely(void)
+{
+	asm volatile (
+	"r6 = *(u32 *)(r1 + 0); "
+	"r6 &= 1; "
+	"r0 = 0; "
+	"if r6 != 0 goto "
+	"+"
+	"2; "
+	" *(u64 *)(r10 - 8) = r0; "
+	"goto "
+	"+"
+	"2; "
+	"r0 = 1; "
+	" *(u64 *)(r10 - 8) = r0; "
+	"r0 = 0; "
+	"r2 = *(u32 *)(r1 + 4); "
+	"r2 &= 7; "
+	"r2 -= 8; "
+	"r2 += r10; "
+	"r3 = *(u8 *)(r2 + 0); "
+	"r1 = r10; "
+	"r1 += -1; "
+	"r1 += r3; "
+	" *(u8 *)(r1 + 0) = 0; "
+	"r0 = 0; "
+	"exit; "
+	::
+	: __clobber_all);
+}
+
 SEC("cgroup/skb")
 __description("variable-offset stack read, uninitialized")
 __success

---
base-commit: ddd664bbff63e09e7a7f9acae9c43605d4cf185f
change-id: 20260610-bpf-stack-var-off-zero-v1-34ad1bc3b533

Best regards,
--  
Woojin Ji <random6.xyz@gmail.com>


             reply	other threads:[~2026-06-11 10:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-11 10:11 Woojin Ji [this message]
2026-06-11 23:43 ` Emil Tsalapatis
2026-06-12 22:27 ` Eduard Zingerman
2026-06-13  0:13 ` Eduard Zingerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260611-bpf-stack-var-off-zero-v1-v1-1-0ec407376147@gmail.com \
    --to=random6.xyz@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®