Re: [PATCH bpf-next v1 01/18] bpf: Add accessors for verifier stack slots

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Wed Sep 23, 2026 at 9:57 PM CEST, bot+bpf-ci wrote:
>> bpf: Add accessors for verifier stack slots
>>
>> The verifier indexes a frame's stack state directly through
>> state->stack[spi] and computes the number of tracked slots as
>> allocated_stack / BPF_REG_SIZE in every file that touches stack slots.
>> Route all of these through two helpers, bpf_stack_slot() and
>> bpf_stack_nr_slots(), so the layout of the per-frame stack state is
>> visible in one place and can change without touching every user.
>> Both take a const frame: the slot accessor returns the slot through the
>> frame's stack pointer, so read-only code such as the state printer can
>> use it without giving up its qualifiers. Functions that look up the
>> same slot repeatedly now fetch it once.
>>
>> No functional change.
>>
>> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> The commit message makes two universal claims that the tree does not back
> up after this patch.
>
> 1) "Route all of these through two helpers ... so the layout of the
> per-frame stack state is visible in one place and can change without
> touching every user."
>
> At least two users of struct bpf_func_state::stack still depend on the raw
> layout:
>
> drivers/net/ethernet/netronome/nfp/bpf/verifier.c:nfp_bpf_map_update_value_ok():
>
>     state = env->cur_state->frame[reg3->frameno];
>     ...
>     soff = -(off + i) - 1;
>     stack_entry = &state->stack[soff / BPF_REG_SIZE];
>

This is correct, will fix.

> This is the exact state->stack[spi] indexing the message says was removed.
> It was not converted.
>
> kernel/bpf/diagnostics.c:reg_to_target(), which this patch edits (the nslots
> line), still does raw pointer arithmetic that assumes stack is a flat array:
>
>     start = (unsigned long)state->stack;
>     end = (unsigned long)(state->stack + nslots);
>     ...
>     spi = ((const char *)reg - (const char *)state->stack) /
>           sizeof(*state->stack);
>
> If bpf_stack_slot() ever stops being &frame->stack[spi], both sites will
> silently compute wrong slots. So the stated goal (the layout can change
> without touching every user) does not hold yet.
>
> Nothing later in 277f2757dee8..7757cddf610a changes the helpers, the layout,
> or the nfp code, so this is not a runtime bug in the series. It is an
> incomplete conversion, or a message that needs narrowing.
>

Yep, also needs fixing.

> 2) "Functions that look up the same slot repeatedly now fetch it once."
>
> Several converted functions still call the accessor twice for the same
> index:
>
> bpf_get_spilled_reg():
>
>     (1 << bpf_stack_slot(frame, slot)->slot_type[BPF_REG_SIZE - 1]) & mask)
>     return &bpf_stack_slot(frame, slot)->spilled_ptr;
>
> bpf_mark_all_scalars_precise():
>
>     if (!bpf_is_spilled_reg(bpf_stack_slot(func, j)))
>     reg = &bpf_stack_slot(func, j)->spilled_ptr;
>
> bpf_mark_chain_precision():
>
>     if (!bpf_is_spilled_scalar_reg(bpf_stack_slot(func, i))) {
>     reg = &bpf_stack_slot(func, i)->spilled_ptr;
>
> The same pattern appears in states.c (propagate_precision,
> iter_active_depths_differ, mark_all_scalars_imprecise) and verifier.c
> (check_stack_write_var_off, get_constant_map_key, collect_linked_regs,
> widen_imprecise_scalars). Only some functions, such as
> print_verifier_state(), were changed to fetch once.
>

This is less serious, but will fix.

> Should nfp_bpf_map_update_value_ok() be converted to bpf_stack_slot(), and
> reg_to_target() given a note or helper for its address-range check?
> Otherwise the changelog could be narrowed to match what the patch actually
> does.
>
>
> ---
> 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/35909853278