Re: [PATCH bpf] bpf: Fix wrong frame passed to check_fastcall_stack_contract() in check_stack_read_fixed_off()

[bot+bpf-ci]

> commit ff6e19d1f0d8e4001693ac46c23ec7204912e5d2
> Author: Ömer Mete Kaya <omermetekaya0@xxxxxxxxx>
> Date:   Mon Sep 28 01:23:44 2026 +0300
>
>     bpf: Fix wrong frame passed to check_fastcall_stack_contract() in check_stack_read_fixed_off()
>
>     check_stack_read_fixed_off() calls check_fastcall_stack_contract() with
>     'state' (the current frame, i.e. the callee) instead of 'reg_state'
>     (the frame that owns the stack being read, i.e. the caller).
>
>     In a bpf2bpf call where a callee reads from a PTR_TO_STACK pointing
>     into the caller's frame, 'reg_state' and 'state' are different:
>     'reg_state' is the caller's frame, while 'state' is the callee's.
>     Passing 'state' causes the fastcall contract to be checked against the
>     callee's subprog instead of the caller's, leaving the caller's fastcall
>     region unprotected and potentially can cause an incorrect fastcall
>     rewrite.
>
>     The three sibling functions are all correct:
>     check_stack_write_fixed_off  -> passes 'state'     (writes to curframe)
>     check_stack_write_var_off    -> passes 'state'     (writes to curframe)
>     check_stack_read_var_off     -> passes 'ptr_state' (reads from ptr's frame)

The explanation of why the write siblings are correct may not be accurate.
The commit message says they pass 'state' because they "write to
curframe", but looking at check_stack_write_fixed_off(), 'state' is
actually computed as the frame the destination pointer refers to:

    struct bpf_func_state *state = bpf_func(env, reg);

which is cur->frame[reg->frameno], not necessarily the current frame. The
function documents the parameter as "stack frame we're writing to" and
compares it against the real current frame separately:

    cur = env->cur_state->frame[env->cur_state->curframe];
    ...
    if (state != cur && reg->type == PTR_TO_STACK) {
        verbose(env, "cannot spill pointers to stack into stack frame of the caller\n");

check_stack_write_var_off() likewise documents 'state' as "func where
register points to".

The existing selftest bad_write_in_subprog in verifier_bpf_fastcall.c has
a callee write through a caller stack pointer, with the comment
"invalidates bpf_fastcall contract for caller". So the write paths already
pass the pointer's frame, just like check_stack_read_var_off() passes
ptr_state.

Could the explanation be updated to say the write helpers pass 'state'
because it's the frame the pointer refers to, not because they write to
curframe? The code change itself is correct, but the "(writes to
curframe)" wording might mislead future readers about what these functions
actually do.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 41b49c56e123..7dd8e32420fd 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3881,7 +3881,7 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
>  	reg = &reg_state->stack[spi].spilled_ptr;
>
>  	mark_stack_slot_scratched(env, spi);
> -	check_fastcall_stack_contract(env, state, env->insn_idx, off);
> +	check_fastcall_stack_contract(env, reg_state, env->insn_idx, off);
>
>  	/*
>  	 * Refine the in-progress load record's origin to the source stack slot.


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