BPF List
 help / color / mirror / Atom feed
* [PATCH bpf 1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO
@ 2026-08-27 22:48 Eduard Zingerman
  2026-08-27 22:48 ` [PATCH bpf 2/2] selftests/bpf: half-dead scalar zero stack spill test Eduard Zingerman
  2026-08-28 17:50 ` [PATCH bpf 1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Eduard Zingerman @ 2026-08-27 22:48 UTC (permalink / raw)
  To: bpf, ast, andrii
  Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor,
	npc

states.c:__clean_func_state() can downgrade scalar zero spill to
STACK_ZERO in the following case:

  *(u64 *)(r10 - 8) = 0;
  ... checkpoint ...
  r1 = *(u32 *)(r10 - 4);
  ... no reads from r10-8 ...

Here 4 bytes at r10-8 are dead and verifier changes scalar spill to a
combination: 0000pppp (p stands for poison). Such a change breaks
precision propagation chains. All places that produce STACK_ZERO
should call bpf_mark_chain_precision() for the zero source.

This patch fixes the bug in a simplest way possible:
avoids converting stack spills of zero to STACK_ZERO.
Two smarter approaches are possible:
- do bpf_mark_chain_precision() from __clean_func_state()
- check slot liveness information in check_stack_write_fixed_off()

I investigated both and the changes required are a bit tricky,
hence go with a simple fix for the time being.

Fixes: be23266b4a08 ("bpf: 4-byte precise clean_verifier_state")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/states.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 4e6aafad33bd..66fb11b6c6a7 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -445,22 +445,19 @@ static void __clean_func_state(struct bpf_verifier_env *env,
 				struct bpf_reg_state *spill = &st->stack[i].spilled_ptr;
 
 				if (lo_live && stype == STACK_SPILL) {
-					u8 val = STACK_MISC;
-
 					if (spill->type != SCALAR_VALUE)
 						continue;
-
 					/*
-					 * 8 byte spill of scalar 0 where half slot is dead
-					 * should become STACK_ZERO in lo 4 bytes.
+					 * Can't replace with STACK_ZERO, because
+					 * that requires bpf_mark_chain_precision().
 					 */
 					if (bpf_register_is_null(spill))
-						val = STACK_ZERO;
+						continue;
 					for (j = 0; j < 4; j++) {
 						u8 *t = &st->stack[i].slot_type[j];
 
 						if (*t == STACK_SPILL)
-							*t = val;
+							*t = STACK_MISC;
 					}
 				}
 				bpf_mark_reg_not_init(env, spill);

-- 
2.55.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH bpf 2/2] selftests/bpf: half-dead scalar zero stack spill test
  2026-08-27 22:48 [PATCH bpf 1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO Eduard Zingerman
@ 2026-08-27 22:48 ` Eduard Zingerman
  2026-08-28 17:50 ` [PATCH bpf 1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Eduard Zingerman @ 2026-08-27 22:48 UTC (permalink / raw)
  To: bpf, ast, andrii
  Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor,
	npc

A test case demonstrating unsafe pruning when spill of a scalar zero
spilled on a first pass in replaced by STACK_ZERO in the
__clean_func_state().

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../selftests/bpf/progs/verifier_spill_fill.c      | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
index 8b166c42c4e0..39a1766dae3f 100644
--- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
+++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
@@ -1403,6 +1403,46 @@ __naked void partial_fill_from_cleaned_pointer_spill(void)
 		      ::: __clobber_all);
 }
 
+SEC("raw_tp")
+__failure
+__msg("access may be outside object bounds")
+__flag(BPF_F_TEST_STATE_FREQ)
+__naked void imprecise_scalar_spill_half_dead(void)
+{
+	asm volatile (
+	/*
+	 * Fork two paths: the one explored first spills an imprecise zero,
+	 * the one explored second, an imprecise non-zero scalar.
+	 */
+	"call %[bpf_get_prandom_u32];"
+	"if r0 > 42 goto 1f;"
+	"r6 = 0;"
+	"goto 2f;"
+"1:"
+	/* causes out of bounds access on a second path. */
+	"r6 = 100500;"
+"2:"
+	/* Force a checkpoint before the spill. */
+	"goto +0;"
+	"*(u64 *)(r10 - 8) = r6;"
+	/*
+	 * Force stack cleanup, only the low half of the spill is alive,
+	 * so the dead high half is degraded to raw stack bytes.
+	 * Buggy verifier converted it to STACK_ZERO w/o proper precision propagation.
+	 */
+	"goto +0;"
+	"r7 = *(u32 *)(r10 - 4);"
+	/* Use r7 as an offset into a one-byte buffer. */
+	"r1 = %[single_byte_buf] ll;"
+	"r1 += r7;"
+	"r0 = *(u8 *)(r1 + 0);"
+	"exit;"
+:
+: __imm(bpf_get_prandom_u32),
+  __imm_addr(single_byte_buf)
+: __clobber_all);
+}
+
 /* check valid spill/fill, ptr to tp buffer */
 SEC("raw_tracepoint.w")
 __success

-- 
2.55.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf 1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO
  2026-08-27 22:48 [PATCH bpf 1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO Eduard Zingerman
  2026-08-27 22:48 ` [PATCH bpf 2/2] selftests/bpf: half-dead scalar zero stack spill test Eduard Zingerman
@ 2026-08-28 17:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-28 17:50 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
	memxor, npc

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 27 Aug 2026 15:48:23 -0700 you wrote:
> states.c:__clean_func_state() can downgrade scalar zero spill to
> STACK_ZERO in the following case:
> 
>   *(u64 *)(r10 - 8) = 0;
>   ... checkpoint ...
>   r1 = *(u32 *)(r10 - 4);
>   ... no reads from r10-8 ...
> 
> [...]

Here is the summary with links:
  - [bpf,1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO
    https://git.kernel.org/bpf/bpf/c/2f3536bff882
  - [bpf,2/2] selftests/bpf: half-dead scalar zero stack spill test
    https://git.kernel.org/bpf/bpf/c/c6ff14f1cd9e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-28 17:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 22:48 [PATCH bpf 1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO Eduard Zingerman
2026-08-27 22:48 ` [PATCH bpf 2/2] selftests/bpf: half-dead scalar zero stack spill test Eduard Zingerman
2026-08-28 17:50 ` [PATCH bpf 1/2] bpf: don't downgrade half-dead scalar zero spills to STACK_ZERO patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox