[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c7e2a776-52f9-46ad-8422-3a9202bbd9f1@linux.dev>
Date: Tue, 13 Jan 2026 15:48:46 -0800
From: Ihor Solodrai <ihor.solodrai@...ux.dev>
To: Eduard Zingerman <eddyz87@...il.com>, Alexei Starovoitov
<ast@...nel.org>, Andrii Nakryiko <andrii@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Martin KaFai Lau <martin.lau@...ux.dev>
Cc: Mykyta Yatsenko <yatsenko@...a.com>, Tejun Heo <tj@...nel.org>,
Alan Maguire <alan.maguire@...cle.com>,
Benjamin Tissoires <bentiss@...nel.org>, Jiri Kosina <jikos@...nel.org>,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-input@...r.kernel.org, sched-ext@...ts.linux.dev
Subject: Re: [PATCH bpf-next v1 03/10] bpf: Verifier support for
KF_IMPLICIT_ARGS
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?
> [...]
Powered by blists - more mailing lists