Re: [PATCH bpf-next v1 03/10] bpf: Verifier support for KF_IMPLICIT_ARGS

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 1/13/26 4:55 PM, Alexei Starovoitov wrote:
> On Tue, Jan 13, 2026 at 3:48 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> On 1/13/26 2:03 PM, Ihor Solodrai wrote:
>>> On 1/13/26 12:39 PM, Eduard Zingerman wrote:
>>>> On Fri, 2026-01-09 at 10:48 -0800, Ihor Solodrai wrote:
>>>>>
>>>>
>>>> [...]
>>>>
>>>>> @@ -14303,6 +14358,17 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>>>>     for (i = 0; i < nargs; i++) {
>>>>>             u32 regno = i + 1;
>>>>>
>>>>> +           /*
>>>>> +            * Implicit kfunc arguments are set after main verification pass.
>>>>> +            * For correct tracking of zero-extensions we have to reset subreg_def for such
>>>>> +            * args. Otherwise mark_btf_func_reg_size() will be inspecting subreg_def of regs
>>>>> +            * from an earlier (irrelevant) point in the program, which may lead to an error
>>>>> +            * in opt_subreg_zext_lo32_rnd_hi32().
>>>>> +            */
>>>>> +           if (unlikely(KF_IMPLICIT_ARGS & meta.kfunc_flags
>>>>> +                           && is_kfunc_arg_implicit(desc_btf, &args[i])))
>>>>> +                   regs[regno].subreg_def = DEF_NOT_SUBREG;
>>>>> +
>>>>
>>>> Did you try doing this in `mark_reg_not_init()`?
>>>> This function is called for R1-R5 some time prior this hunk.
>>>
>>>> Did you try doing this in `mark_reg_not_init()`?
>>>
>>> Just tried, it doesn't work because REG0 is considered a caller saved
>>> register, and so it breaks the zext tracking:
>>>
>>>         #define CALLER_SAVED_REGS 6
>>>         static const int caller_saved[CALLER_SAVED_REGS] = {
>>>            BPF_REG_0, BPF_REG_1, BPF_REG_2, BPF_REG_3, BPF_REG_4, BPF_REG_5
>>>         };
>>>
>>>         [...]
>>>
>>>       for (i = 0; i < CALLER_SAVED_REGS; i++)
>>>               mark_reg_not_init(env, regs, caller_saved[i]);
>>>
>>> CI run for the diff below (on top of this series):
>>> https://github.com/kernel-patches/bpf/actions/runs/20972520708
>>>
>>>
>>> [...]
>>>
>>> ---
>>>
>>> Resetting all reg args appears to be working however (see below).
>>> CI: https://github.com/kernel-patches/bpf/actions/runs/20973490221
>>>
>>
>> A follow up after a chat with Eduard.
>>
>> This change in check_kfunc_call() appears to be working:
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 092003cc7841..ff743335111c 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -13958,8 +13958,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>                 regs = branch->frame[branch->curframe]->regs;
>>
>>                 /* Clear r0-r5 registers in forked state */
>> -               for (i = 0; i < CALLER_SAVED_REGS; i++)
>> -                       mark_reg_not_init(env, regs, caller_saved[i]);
>> +               for (i = 0; i < CALLER_SAVED_REGS; i++) {
>> +                       u32 regno = caller_saved[i];
>> +                       mark_reg_not_init(env, regs, regno);
>> +                       regs[regno].subreg_def = DEF_NOT_SUBREG;
>> +               }
>>
>>                 mark_reg_unknown(env, regs, BPF_REG_0);
>>                 err = __mark_reg_s32_range(env, regs, BPF_REG_0, -MAX_ERRNO, -1);
>>
>> https://github.com/kernel-patches/bpf/actions/runs/20975419422
>>
>> Apparently, doing .subreg_def = DEF_NOT_SUBREG in mark_reg_not_init()
>> breaks zero-extension tracking somewhere else.  But this is not
>> directly relevant to the series.
>>
>> Eduard, Alexei, any concerns with this diff? Should I send a separate
>> patch?
> 
> This is odd. Clear it only for res_spin_lock() processing?!
> Should be around lines 14149 instead?

Yes. Sorry, this was a messed up local diff. The commits tested on CI
are correct though. I'll have this fix in v2, since it is necessary
for KF_IMPLICIT_ARGS to work.

I'll look into this problem more after implicit args land, unless
someone beats me to it.

> 
> First, need to investigate why clearing it in mark_reg_not_init()
> breaks things.
> That's what clear_caller_saved_regs() is doing already.
> Maybe these two loops in check_kfunc_call() should be doing
> clear_caller_saved_regs() instead...
> Needs proper investigation.