[PATCH bpf 0/2] bpf: Keep fastcall stack slots for helper stack reads

[PATCH bpf 0/2] bpf: Keep fastcall stack slots for helper stack reads

[Nuoqi Gui]

Fastcall spill/fill elision is guarded by a stack contract: stack slots in
the pattern may only be accessed by the pattern itself. Direct stack loads
and stores enforce that contract, but helper and kfunc memory arguments can
read from PTR_TO_STACK through check_stack_range_initialized() without
disabling the post-verification elision.

Make helper/kfunc stack memory checks enforce the fastcall contract after
resolving the range. Add a verifier selftest for a read-only helper access
through bpf_csum_diff().

Fixes: 5b5f51bff1b66 ("bpf: no_caller_saved_registers attribute for helper calls")

Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
---
Nuoqi Gui (2):
      bpf: Keep fastcall spills for helper stack reads
      selftests/bpf: Cover fastcall helper stack reads

 kernel/bpf/verifier.c                              |  4 +++
 .../selftests/bpf/progs/verifier_bpf_fastcall.c    | 32 ++++++++++++++++++++++
 2 files changed, 36 insertions(+)
---
base-commit: 76f62d237538b456354a44e796a541cde03c6e28
change-id: 20260624-f01-12-fastcall-helper-stack-read-6d4dc1ffb513

Best regards,
--  
Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

[PATCH bpf 1/2] bpf: Keep fastcall spills for helper stack reads

[Nuoqi Gui]

The fastcall spill/fill rewrite is only sound while the stack slots used by
the pattern are not accessed outside the pattern. Direct stack loads and
stores already call check_fastcall_stack_contract() to enforce this.

Helper and kfunc memory-argument checks can validate PTR_TO_STACK reads
through check_stack_range_initialized() without applying the same contract.
When such a read overlaps a fastcall spill slot,
bpf_remove_fastcall_spills_fills() can still remove the spill/fill pair.
It can then shrink the subprogram stack depth even though a helper or kfunc
reads that stack address.

Apply check_fastcall_stack_contract() from check_stack_range_initialized()
after the concrete stack range is known. Zero-sized accesses do not read or
write memory, so leave the fastcall optimization unchanged for those.

Fixes: 5b5f51bff1b66 ("bpf: no_caller_saved_registers attribute for helper calls")
Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
---
 kernel/bpf/verifier.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ff9b1f68ceca4..d77332eeab359 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6873,6 +6873,10 @@ static int check_stack_range_initialized(
 		max_off = reg->smax_value + off;
 	}
 
+	if (access_size)
+		check_fastcall_stack_contract(env, state, env->insn_idx,
+					      min_off);
+
 	if (meta && meta->raw_mode) {
 		/* Ensure we won't be overwriting dynptrs when simulating byte
 		 * by byte access in check_helper_call using meta.access_size.

-- 
2.34.1

Re: [PATCH bpf 1/2] bpf: Keep fastcall spills for helper stack reads

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] check_stack_read_fixed_off passes the wrong execution frame to check_fastcall_stack_contract
- [High] fastcall_stack_off minimum bound logic leaves longer fastcall patterns unprotected
--

commit 8cfed53d5c0d5c6188360232f775d8b94b125599
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

bpf: Keep fastcall spills for helper stack reads

The fastcall spill/fill rewrite is only sound while the stack slots used by
the pattern are not accessed outside the pattern. Direct stack loads and
stores already call check_fastcall_stack_contract() to enforce this.

Helper and kfunc memory-argument checks can validate PTR_TO_STACK reads
through check_stack_range_initialized() without applying the same contract.
When such a read overlaps a fastcall spill slot,
bpf_remove_fastcall_spills_fills() can still remove the spill/fill pair.
It can then shrink the subprogram stack depth even though a helper or kfunc
reads that stack address.

Apply check_fastcall_stack_contract() from check_stack_range_initialized()
after the concrete stack range is known. Zero-sized accesses do not read or
write memory, so leave the fastcall optimization unchanged for those.

Fixes: 5b5f51bff1b66 ("bpf: no_caller_saved_registers attribute for helper calls")
Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 69d75515ed3f7..592da530bdc5a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6874,6 +6874,10 @@ static int check_stack_range_initialized(
>  		max_off = reg->smax_value + off;
>  	}
>  
> +	if (access_size)
> +		check_fastcall_stack_contract(env, state, env->insn_idx,
> +					      min_off);
> +

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this change I
noticed a pre-existing issue in check_stack_read_fixed_off() with how it
passes execution frames to check_fastcall_stack_contract().

In check_stack_read_fixed_off():

	struct bpf_verifier_state *vstate = env->cur_state;
	struct bpf_func_state *state = vstate->frame[vstate->curframe];
	...
	check_fastcall_stack_contract(env, state, env->insn_idx, off);

Here, the local state variable evaluates to the current execution frame
(the callee), instead of reg_state which is the frame where the stack memory
actually resides (and can be the caller).

If a BPF subprogram is passed a pointer to its caller's stack and reads a
stack slot that overlaps with a fastcall spill slot in the caller, couldn't
the fastcall bounds be checked against the wrong subprogram's metadata?

Could this cause the verifier to fail to disable the fastcall rewrite for
the caller, resulting in uninitialized stack memory reads when
bpf_remove_fastcall_spills_fills() removes the caller's spill instructions?

[Severity: High]
This is a pre-existing issue, but I also noticed a potential problem in
mark_fastcall_pattern_for_call() with how it tracks fastcall stack boundaries:

	subprog->fastcall_stack_off = min(subprog->fastcall_stack_off, off);

Because off is a negative stack offset that moves closer to 0 for longer
patterns, using min() incorrectly shrinks the protected stack region to the
upper bound of the shortest fastcall pattern.

If a BPF subprogram contains multiple fastcall helper patterns of different
lengths (for example, Call A with 1 spill ending at offset -24, and Call B
with 2 spills ending at offset -16), couldn't this expose the upper spill
slots of any longer fastcall patterns?

If a program performs a direct memory access to the highest spill slot (-16)
of the longer pattern outside of the fastcall block, the verifier might
mistakenly preserve the fastcall optimization, removing the spills and
causing uninitialized memory reads at runtime.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-f01-12-fastcall-helper-stack-read-v1-0-e0a86085ef77@mails.tsinghua.edu.cn?part=1

[PATCH bpf 2/2] selftests/bpf: Cover fastcall helper stack reads

[Nuoqi Gui]

Add verifier_bpf_fastcall test where a spill/fill pair initializes a stack
slot. bpf_csum_diff() later reads it through ARG_PTR_TO_MEM | MEM_RDONLY.

The translated program must keep the spill and fill materialized. Otherwise
the helper reads stack bytes that no longer match the verifier model.

Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
---
 .../selftests/bpf/progs/verifier_bpf_fastcall.c    | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c b/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
index fb4fa465d67c6..aab92e69673d2 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
@@ -500,6 +500,38 @@ __naked void bad_helper_write(void)
 	: __clobber_all);
 }
 
+SEC("tc")
+__arch_x86_64
+__xlated("1: *(u64 *)(r10 -8) = r1")
+__xlated("...")
+__xlated("3: r0 = &(void __percpu *)(r0)")
+__xlated("...")
+__xlated("5: r1 = *(u64 *)(r10 -8)")
+__success
+__naked void bad_helper_read(void)
+{
+	asm volatile (
+	"r1 = 1;"
+	/* bpf_fastcall pattern with stack offset -8 */
+	"*(u64 *)(r10 - 8) = r1;"
+	"call %[bpf_get_smp_processor_id];"
+	"r1 = *(u64 *)(r10 - 8);"
+	/* bpf_csum_diff() reads fp[-8], so keep the spill materialized */
+	"r1 = r10;"
+	"r1 += -8;"
+	"r2 = 8;"
+	"r3 = 0;"
+	"r4 = 0;"
+	"r5 = 0;"
+	"call %[bpf_csum_diff];"
+	"r0 = 0;"
+	"exit;"
+	:
+	: __imm(bpf_get_smp_processor_id),
+	  __imm(bpf_csum_diff)
+	: __clobber_all);
+}
+
 SEC("raw_tp")
 __arch_x86_64
 /* main, not patched */

-- 
2.34.1

Re: [PATCH bpf 2/2] selftests/bpf: Cover fastcall helper stack reads

[bot+bpf-ci]

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

>     selftests/bpf: Cover fastcall helper stack reads
>
>     Add verifier_bpf_fastcall test where a spill/fill pair initializes a stack
>     slot. bpf_csum_diff() later reads it through ARG_PTR_TO_MEM | MEM_RDONLY.
>
>     The translated program must keep the spill and fill materialized. Otherwise
>     the helper reads stack bytes that no longer match the verifier model.
>
>     Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

This test covers a fastcall spill/fill removal bug that the bpf_fastcall
spill/fill removal mechanism introduced in commit 5b5f51bff1b66
("bpf: no_caller_saved_registers attribute for helper calls").

Should this commit carry a Fixes: tag pointing at that commit?

  Fixes: 5b5f51bff1b66 ("bpf: no_caller_saved_registers attribute for helper calls")


---
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/28089002726