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

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