> -----Original Messages-----
> From: "Eduard Zingerman" <[email protected]>
> Send time:Friday, 26/06/2026 02:32:32
> To: "Nuoqi Gui" <[email protected]>, "Alexei Starovoitov"
> <[email protected]>, "Daniel Borkmann" <[email protected]>, "Andrii
> Nakryiko" <[email protected]>, "Kumar Kartikeya Dwivedi" <[email protected]>
> Cc: "John Fastabend" <[email protected]>, "Martin KaFai Lau"
> <[email protected]>, "Shuah Khan" <[email protected]>,
> [email protected], [email protected],
> [email protected]
> Subject: Re: [PATCH bpf 1/2] bpf: Keep fastcall spills for helper stack reads
>
> On Wed, 2026-06-24 at 16:39 +0800, Nuoqi Gui wrote:
> > 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 <[email protected]>
> > ---
> > 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);
> > +
>
> Thank you for finding this bug.
> I think that placing the check_fastcall_stack_contract() call here
> makes the same call unnecessary in the check_stack_read_var_off(),
> as it calls to check_stack_range_initialized() with zero_size_allowed==false.
>
> > 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.
Thanks. I'll remove the redundant call from check_stack_read_var_off() in v2.
