Re: [PATCH bpf-next v3 05/24] bpf: Support stack arguments for bpf functions

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

On 5/11/26 8:19 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index 321b9d69cf9c..f9020a4ea005 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -402,6 +402,7 @@ struct bpf_func_state {
>>   	bool in_callback_fn;
>>   	bool in_async_callback_fn;
>>   	bool in_exception_callback_fn;
>> +	bool no_stack_arg_load;
> [ ... ]
>
>> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
>> index bd9c22945050..c249eb40c6d6 100644
>> --- a/kernel/bpf/states.c
>> +++ b/kernel/bpf/states.c
>> @@ -833,6 +833,32 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
>>   	return true;
>>   }
>>
>> +/*
>> + * Compare stack arg slots between old and current states.
>> + * Outgoing stack args are path-local state and must agree for pruning.
>> + */
>> +static bool stack_arg_safe(struct bpf_verifier_env *env, struct bpf_func_state *old,
>> +			   struct bpf_func_state *cur, struct bpf_idmap *idmap,
>> +			   enum exact_level exact)
>> +{
>> +	int i, nslots;
>> +
>> +	nslots = max(old->out_stack_arg_cnt, cur->out_stack_arg_cnt);
>> +	for (i = 0; i < nslots; i++) {
>> +		struct bpf_reg_state *old_arg, *cur_arg;
>> +		struct bpf_reg_state not_init = { .type = NOT_INIT };
>> +
>> +		old_arg = i < old->out_stack_arg_cnt ?
>> +			  &old->stack_arg_regs[i] : &not_init;
>> +		cur_arg = i < cur->out_stack_arg_cnt ?
>> +			  &cur->stack_arg_regs[i] : &not_init;
>> +		if (!regsafe(env, old_arg, cur_arg, idmap, exact))
>> +			return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>   static bool refsafe(struct bpf_verifier_state *old, struct bpf_verifier_state *cur,
>>   		    struct bpf_idmap *idmap)
>>   {
>> @@ -924,6 +950,9 @@ static bool func_states_equal(struct bpf_verifier_env *env, struct bpf_func_stat
>>   	if (!stacksafe(env, old, cur, &env->idmap_scratch, exact))
>>   		return false;
>>
>> +	if (!stack_arg_safe(env, old, cur, &env->idmap_scratch, exact))
>> +		return false;
>> +
>>   	return true;
>>   }
> In v2 (May 7), bot+bpf-ci@kernel.org raised a concern that
> no_stack_arg_load is not compared during state pruning in
> func_states_equal():
>
>      "The same issue applies to state pruning: func_states_equal/
>      stack_arg_safe does not compare no_stack_arg_load either, so a
>      state with no_stack_arg_load=true can be pruned to an old state
>      with no_stack_arg_load=false incorrectly. Should no_stack_arg_load
>      be placed before stack (to be memcpy'd) or should copy_stack_state()
>      explicitly assign it?"
>
> The field placement was fixed (moving no_stack_arg_load before 'stack'
> so it gets copied by memcpy), but func_states_equal() still does not
> explicitly compare this field.
>
> Could this allow incorrect pruning? If a BPF program writes to stack
> args and calls a function on one path (setting no_stack_arg_load=true),
> then branches and tries to read from stack args on another path, the
> verifier might incorrectly prune to the old state with
> no_stack_arg_load=false, missing the enforcement that stack arg reads
> must happen before writes/calls.
>
> Looking at func_states_equal():
>
> kernel/bpf/states.c:func_states_equal() {
>      if (!stacksafe(env, old, cur, &env->idmap_scratch, exact))
>          return false;
>
>      if (!stack_arg_safe(env, old, cur, &env->idmap_scratch, exact))
>          return false;
>
>      return true;
> }
>
> Should no_stack_arg_load be compared here, similar to how other boolean
> flags in bpf_func_state are handled during state comparison?

You are right. The following is an example:

   /* subprog with incoming stack arg */
   r1 = *(r11+8);         /* read incoming arg, no_stack_arg_load = false */
                                                                                                                    
   if cond goto path2;
                                                                                                                    
   path1:
     call some_helper;    /* sets no_stack_arg_load = true */
     goto join;
                                                                                                                    
   path2:
     /* no call, no_stack_arg_load = false */
     goto join;
                                                                                                                    
   join:
     r2 = *(r11+8);       /* read incoming arg again */

In the above case, at 'join' point, we have no_stack_arg_load = true and false
respectively. In this case, we cannot do pruning.

Will fix.

>
> Reference: https://lore.kernel.org/bpf/11c51daec78a68837f719172ae1c21db8b3e98e0a76aa5bcd59c8b089760c40d@mail.kernel.org/
>
>
> ---
> 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/25652623893