[PATCH bpf-next 0/2] bpf: Validate byte type for helper stack reads

[PATCH bpf-next 0/2] bpf: Validate byte type for helper stack reads

[Yiyang Chen]

The verifier checks helper memory arguments byte by byte, but its scalar
spill fast path only checks the slot-wide spill marker. For a CAP_BPF load,
a narrow scalar spill can therefore make an untouched sibling byte appear
initialized when the sibling is passed to a helper.

Patch 1 makes the fast path require STACK_SPILL for the byte being checked.
Patch 2 adds verifier_spill_fill coverage for a CAP_BPF map update helper
reading an invalid sibling byte.

Yiyang Chen (2):
  bpf: Check byte type when validating helper stack reads
  selftests/bpf: Cover helper reads of invalid spill siblings

 kernel/bpf/verifier.c                         |  3 +-
 .../selftests/bpf/progs/verifier_spill_fill.c | 33 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)


base-commit: 9a3a07d06e7d74f4aecc51396c771149336ac55d
-- 
2.34.1

[PATCH bpf-next 1/2] bpf: Check byte type when validating helper stack reads

[Yiyang Chen]

check_stack_range_initialized() validates helper ranges byte by byte, but
bpf_is_spilled_reg() only checks the slot-wide spill marker. For a CAP_BPF
load, a narrow scalar spill marks only the covered bytes as STACK_SPILL. An
untouched sibling remains STACK_INVALID, but the helper still accepts the
read when the slot contains a spilled scalar.

Require the byte being checked to be STACK_SPILL before accepting the
slot-wide scalar spill state.

Fixes: 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill")
Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
---
 kernel/bpf/verifier.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 03e2202cca13d..c971cf34c760b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6785,7 +6785,8 @@ static int check_stack_range_initialized(
 			goto mark;
 		}
 
-		if (bpf_is_spilled_reg(&state->stack[spi]) &&
+		if (*stype == STACK_SPILL &&
+		    bpf_is_spilled_reg(&state->stack[spi]) &&
 		    (state->stack[spi].spilled_ptr.type == SCALAR_VALUE ||
 		     env->allow_ptr_leaks)) {
 			if (clobber) {
-- 
2.34.1

[PATCH bpf-next 2/2] selftests/bpf: Cover helper reads of invalid spill siblings

[Yiyang Chen]

Add a verifier_spill_fill case that passes an untouched byte next to a
one-byte scalar spill to bpf_map_update_elem(). The full-capability load
retains the existing uninitialized-stack behavior, while the CAP_BPF-only
load must reject the STACK_INVALID helper read.

Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
---
 .../selftests/bpf/progs/verifier_spill_fill.c | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
index 8b166c42c4e0e..e686f6d8d5656 100644
--- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
+++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
@@ -11,6 +11,13 @@ struct {
 	__uint(max_entries, 4096);
 } map_ringbuf SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u8);
+} map_partial_spill SEC(".maps");
+
 SEC("socket")
 __description("check valid spill/fill")
 __success __failure_unpriv __msg_unpriv("R0 leaks addr")
@@ -1307,6 +1314,32 @@ __naked void stack_noperfmon_spill_32bit_onto_64bit_slot(void)
 	: __clobber_all);
 }
 
+SEC("socket")
+__description("helper read must reject an invalid sibling of a narrow spill")
+__success
+__caps_unpriv(CAP_BPF)
+__failure_unpriv __msg_unpriv("invalid read from stack R3 off -7+0 size 1")
+__naked void helper_read_invalid_partial_spill_sibling(void)
+{
+	asm volatile ("r5 = 1;\n\t"
+	"*(u8 *)(r10 - 8) = r5;\n\t"
+	"r2 = 0;\n\t"
+	"*(u32 *)(r10 - 16) = r2;\n\t"
+	"r1 = %[map_partial_spill] ll;\n\t"
+	"r2 = r10;\n\t"
+	"r2 += -16;\n\t"
+	"r3 = r10;\n\t"
+	"r3 += -7;\n\t"
+	"r4 = 0;\n\t"
+	"call %[bpf_map_update_elem];\n\t"
+	"r0 = 0;\n\t"
+	"exit;\n\t"
+	:
+	: __imm(bpf_map_update_elem),
+	  __imm_addr(map_partial_spill)
+	: __clobber_all);
+}
+
 /*
  * stacksafe(): check if 32-bit scalar spill in old state is considered
  * equivalent to STACK_MISC in cur state.
-- 
2.34.1

Re: [PATCH bpf-next 2/2] selftests/bpf: Cover helper reads of invalid spill siblings

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

> selftests/bpf: Cover helper reads of invalid spill siblings
>
>     Add a verifier_spill_fill case that passes an untouched byte next to a
>     one-byte scalar spill to bpf_map_update_elem(). The full-capability load
>     retains the existing uninitialized-stack behavior, while the CAP_BPF-only
>     load must reject the STACK_INVALID helper read.
>
>     Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>

This test covers a verifier bug where a helper read accepted an
uninitialized (STACK_INVALID) sibling byte next to a narrow (<8-byte) scalar
spill.  That behavior came in when sub-8-byte scalar spill/refill support was
added, which left sibling bytes STACK_INVALID while the slot still reported a
spilled scalar.

Should this carry the same Fixes: tag as the companion verifier fix so stable
backports pull the test in alongside it?

  Fixes: 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill")

> diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
> index 8b166c42c4e0e..e686f6d8d5656 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c

[ ... ]

> @@ -1307,6 +1314,32 @@ __naked void stack_noperfmon_spill_32bit_onto_64bit_slot(void)
>  	: __clobber_all);
>  }
>
> +SEC("socket")
> +__description("helper read must reject an invalid sibling of a narrow spill")
> +__success
> +__caps_unpriv(CAP_BPF)
> +__failure_unpriv __msg_unpriv("invalid read from stack R3 off -7+0 size 1")
> +__naked void helper_read_invalid_partial_spill_sibling(void)
> +{
> +	asm volatile ("r5 = 1;\n\t"
> +	"*(u8 *)(r10 - 8) = r5;\n\t"
> +	"r2 = 0;\n\t"
> +	"*(u32 *)(r10 - 16) = r2;\n\t"

this isn't a bug, but could this asm block use the same backslash
line-continuation style as the neighbouring tests in this file?  The two
tests just above it, stack_noperfmon_reject_invalid_read() and
stack_noperfmon_spill_32bit_onto_64bit_slot(), write a single string with
trailing tab-backslash, and this is the only "\n\t" per-line block in
verifier_spill_fill.c.

> +	"r1 = %[map_partial_spill] ll;\n\t"
> +	"r2 = r10;\n\t"
> +	"r2 += -16;\n\t"
> +	"r3 = r10;\n\t"
> +	"r3 += -7;\n\t"
> +	"r4 = 0;\n\t"
> +	"call %[bpf_map_update_elem];\n\t"
> +	"r0 = 0;\n\t"
> +	"exit;\n\t"
> +	:
> +	: __imm(bpf_map_update_elem),
> +	  __imm_addr(map_partial_spill)
> +	: __clobber_all);
> +}
>


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/29229349078

Re: [PATCH bpf-next 1/2] bpf: Check byte type when validating helper stack reads

[Eduard Zingerman]

On Mon, 2026-07-13 at 06:16 +0000, Yiyang Chen wrote:
> check_stack_range_initialized() validates helper ranges byte by byte, but
> bpf_is_spilled_reg() only checks the slot-wide spill marker. For a CAP_BPF
> load, a narrow scalar spill marks only the covered bytes as STACK_SPILL. An
> untouched sibling remains STACK_INVALID, but the helper still accepts the
> read when the slot contains a spilled scalar.
> 
> Require the byte being checked to be STACK_SPILL before accepting the
> slot-wide scalar spill state.
> 
> Fixes: 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill")
> Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
> ---
>  kernel/bpf/verifier.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 03e2202cca13d..c971cf34c760b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6785,7 +6785,8 @@ static int check_stack_range_initialized(
>  			goto mark;
>  		}
>  
> -		if (bpf_is_spilled_reg(&state->stack[spi]) &&
> +		if (*stype == STACK_SPILL &&
> +		    bpf_is_spilled_reg(&state->stack[spi]) &&

Sorry, could you please explain again what's wrong with the original check?
Here it is:

                if (bpf_is_spilled_reg(&state->stack[spi]) &&
                    (state->stack[spi].spilled_ptr.type == SCALAR_VALUE || env->allow_ptr_leaks)) {
                        ... accepted ...
                }

And it says: if there is a register spill at 'spi':
- always allow access if the spilled register is scalar
- when allow_ptr_leaks is true, allow access when spilled
  register is either scalar or pointer.

The goal here is to avoid leaking pointers, and pointers are always 8-bytes.

>  		    (state->stack[spi].spilled_ptr.type == SCALAR_VALUE ||
>  		     env->allow_ptr_leaks)) {
>  			if (clobber) {

pw-bot: cr.

Re: [PATCH bpf-next 1/2] bpf: Check byte type when validating helper stack reads

[Yiyang Chen]

On Mon, 2026-07-13 at 21:22 +0000, Eduard Zingerman wrote:
> Sorry, could you please explain again what's wrong with the original check?
> Here it is:
>
>                 if (bpf_is_spilled_reg(&state->stack[spi]) &&
>                     (state->stack[spi].spilled_ptr.type == SCALAR_VALUE || env->allow_ptr_leaks)) {
>                         ... accepted ...
>                 }
>
> And it says: if there is a register spill at 'spi':
> - always allow access if the spilled register is scalar
> - when allow_ptr_leaks is true, allow access when spilled
>   register is either scalar or pointer.
>
> The goal here is to avoid leaking pointers, and pointers are always 8-bytes.

You're right that this block is a pointer-leak check, and a scalar spill
can't leak a pointer -- so the fix isn't justified on those grounds. The
invariant it targets is the other one this function enforces: rejecting
reads of uninitialized stack when !allow_uninit_stack.

The block just above already gates that:

  /* verifier.c:6780 */
  if ((*stype == STACK_ZERO) ||
      (*stype == STACK_INVALID && env->allow_uninit_stack)) { ... goto mark; }

For a CAP_BPF-only load allow_uninit_stack is false, so a STACK_INVALID
byte should reach the -EACCES below. allow_ptr_leaks and allow_uninit_stack
are both keyed on CAP_PERFMON today but are separate flags; reads-of-invalid
is independent of pointer leaks. The existing fix for the related
producer-side bug, 69772f509e08 ("bpf: Don't mark STACK_INVALID as
STACK_MISC in mark_stack_slot_misc"), frames it the same way:

  "we should not upgrade STACK_INVALID to STACK_MISC when allow_ptr_leaks
   is false, since invalid contents shouldn't be read unless the program
   has the relevant capabilities."

After a 1-byte scalar spill to fp-8, save_register_state() marks only the
covered byte STACK_SPILL; the sibling fp-7 goes to mark_stack_slot_misc(),
which (verifier.c:1234) won't promote STACK_INVALID. So slot_type[7]
(fp-8) is STACK_SPILL and slot_type[6] (fp-7) is STACK_INVALID. When the
helper walk reaches fp-7, the uninit gate above fails correctly, but the
spill block accepts it anyway -- bpf_is_spilled_reg() tests
slot_type[BPF_REG_SIZE-1], the fp-8 sibling, not the fp-7 byte being checked.

The patch makes the fast path cover only bytes it actually marked, so the
invalid sibling falls through to the -EACCES the upper gate chose for it.
It's the consumer-side counterpart to 69772f509e08 (producer side) and its
direct-read selftest f513c3635078: the direct read was already covered, the
helper-argument path was not.

Runtime: for a CAP_BPF-only load, passing fp-7 as the value to
bpf_map_update_elem() after the spill to fp-8 exports a byte the program
never wrote. A direct read of fp-7 rejects; only the helper path accepts.

The commit message should say this rather than just restating the mechanism
-- I'll send a v2. Happy to move the fix elsewhere if you'd prefer.