Re: [PATCH bpf-next 03/10] bpf: Support stack arguments for bpf functions

[Yonghong Song <yonghong.song@linux.dev>]

On 4/1/26 8:18 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index 090aa26d1c98..a260610cd1c1 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
> [ ... ]
>
>> @@ -319,6 +324,10 @@ struct bpf_func_state {
>>   	 * `stack`. allocated_stack is always a multiple of BPF_REG_SIZE.
>>   	 */
>>   	int allocated_stack;
>> +
>> +	u16 stack_arg_depth; /* Size of incoming + max outgoing stack args in bytes. */
>> +	u16 incoming_stack_arg_depth; /* Size of incoming stack args in bytes. */
>> +	struct bpf_stack_arg_state *stack_arg_slots;
>>   };
> The new stack_arg_slots state is part of bpf_func_state and is
> properly copied in copy_stack_state() and freed in free_func_state(),
> but stacksafe() does not compare it during state pruning.
>
> stacksafe() only iterates the regular stack:
>
> kernel/bpf/verifier.c:stacksafe() {
>      for (i = 0; i < old->allocated_stack; i++) {
>          ...
>          old->stack[spi].slot_type[...]
>          ...
>      }
>      return true;
> }
>
> It never examines old->stack_arg_slots or cur->stack_arg_slots. This
> means func_states_equal() -> stacksafe() treats two states as
> equivalent even when they differ in their stack arg slot contents.
>
> Could this allow the verifier to incorrectly prune a path where
> outgoing stack arg slots are uninitialized? For example, if a BPF
> program has two paths to a call instruction -- one that writes the
> outgoing stack args and one that does not -- the verifier could
> explore the initialized path first, save the state at a checkpoint,
> then prune the uninitialized path as equivalent. The initialization
> check in set_callee_state():
>
> kernel/bpf/verifier.c:set_callee_state() {
>      for (i = 0; i < callee_incoming_slots; i++) {
>          int caller_spi = i + caller_incoming_slots;
>
>          if (!is_stack_arg_slot_initialized(caller, caller_spi)) {
>              verbose(env, "stack arg#%d not properly initialized\n",
>                  i + 1 + MAX_BPF_FUNC_REG_ARGS);
>              return -EINVAL;
>          }
>          callee->stack_arg_slots[i] = caller->stack_arg_slots[caller_spi];
>      }
> }
>
> would never run on the pruned path, potentially allowing the callee
> to receive uninitialized data.
>
> It seems like stacksafe() needs a companion loop for stack_arg_slots,
> similar to how it walks the regular stack, to maintain verifier
> soundness for this new state.

You are absolutely correct. Indeed, we should do some checking for
stack_arg_slots along side stacksafe(). I will fix it in the next
revision.

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