From: Eduard Zingerman <eddyz87@gmail.com>
To: Ihor Solodrai <ihor.solodrai@linux.dev>, chenyuan_fl@163.com
Cc: andrii@kernel.org, ast@kernel.org, bot+bpf-ci@kernel.org,
bpf@vger.kernel.org, chenyuan@kylinos.cn, clm@meta.com,
daniel@iogearbox.net, jolsa@kernel.org,
linux-kernel@vger.kernel.org, martin.lau@kernel.org,
martin.lau@linux.dev, memxor@gmail.com, song@kernel.org,
yonghong.song@linux.dev
Subject: Re: [PATCH bpf v5 1/2] bpf: Fix kfunc implicit arg inject type detection to prevent invalid pointer deref
Date: Mon, 08 Jun 2026 23:20:36 -0700 [thread overview]
Message-ID: <0d63a8d68bf0778d6e931ed27892a5bcb151295d.camel@gmail.com> (raw)
In-Reply-To: <118b0bc7-5126-465d-993c-3b25e331e2cf@linux.dev>
On Mon, 2026-06-08 at 18:18 -0700, Ihor Solodrai wrote:
[...]
> > > > @@ -11885,9 +11885,27 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> > > > continue;
> > > > }
> > > >
> > > > - if (is_kfunc_arg_ignore(btf, &args[i]) || is_kfunc_arg_implicit(meta, i))
> > > > + if (is_kfunc_arg_ignore(btf, &args[i]))
> > > > continue;
> > > >
> > > > + if (is_kfunc_arg_implicit(meta, i)) {
> > > > + /* kfuncs with implicit args (e.g. 'off' parameter)
> > > > + * handled during verification in bpf_fixup_kfunc_call():
> > > > + * obj_new, percpu_obj_new, obj_drop, percpu_obj_drop,
> > > > + * refcount_acquire, list_push, rbtree_add. Don't flag them. */
> > > > + if (is_bpf_obj_new_kfunc(meta->func_id) ||
> > > > + is_bpf_percpu_obj_new_kfunc(meta->func_id) ||
> > > > + is_bpf_obj_drop_kfunc(meta->func_id) ||
> > > > + is_bpf_percpu_obj_drop_kfunc(meta->func_id) ||
> > > > + is_bpf_refcount_acquire_kfunc(meta->func_id) ||
> > > > + is_bpf_list_push_kfunc(meta->func_id) ||
> > > > + is_bpf_rbtree_add_kfunc(meta->func_id))
> > >
> > > Is the goal here to have a nice error message?
> > >
> > > I think this will fail for other functions like bpf_wq_set_callback().
> > > For a proper check, the list must include every single kfunc with KF_IMPLICIT_ARGS, no?
> > >
> > > If we go this route, then the list of flagged kfuncs can be collected automatically.
> > > I'm not sure we actually want to do this.
> >
> > The calls to functions with implicit arguments are patched by
> > bpf_fixup_kfunc_call(). As far as I understand, this function:
> > - handles functions with implicit bpf_prog_aux generically
> > - handles the functions listed above on a case-by-case basis.
> >
> > The goal is not to have a nice error message, but to prevent runtime
> > from reading garbage from a register.
>
> In check_kfunc_args() we only needed to know whether the arg is implicit,
> independent of its type, which is_kfunc_arg_implicit() already does. To skip it.
>
> For vmlinux kernel kfuncs this should be enough, I think.
>
> To properly harden against reading garbage for a *module* kfunc, I think the
> verifier would have to check for the specific BTF type, e.g. "are we patching a
> struct bpf_prog_aux pointer?".
The only way for a module kfunc can have implicit args is to have
bpf_prog_aux parameter and an implicit args flag.
There are no module-specific callbacks to do custom implicit args patching.
The actual arguments patching is done by verifier.c:bpf_fixup_kfunc_call().
It has the following structure:
int bpf_fixup_kfunc_call(...)
{
...
if (is_bpf_obj_new_kfunc(desc->func_id) || is_bpf_percpu_obj_new_kfunc(desc->func_id)) {
... patch args ...
} else if (is_bpf_obj_drop_kfunc(desc->func_id) ||
is_bpf_percpu_obj_drop_kfunc(desc->func_id) ||
is_bpf_refcount_acquire_kfunc(desc->func_id)) {
... patch args ...
} else if (is_bpf_list_push_kfunc(desc->func_id) ||
is_bpf_rbtree_add_kfunc(desc->func_id)) {
... patch args ...
} else if (desc->func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx] ||
desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) {
...
} else if (desc->func_id == special_kfunc_list[KF_bpf_session_is_return] && ...) {
...
} else if (desc->func_id == special_kfunc_list[KF_bpf_session_cookie] && ...) {
...
}
if (env->insn_aux_data[insn_idx].arg_prog) {
u32 regno = env->insn_aux_data[insn_idx].arg_prog;
... patch bpf_prog_aux arg in `regno` ...
}
return 0;
}
Also consider how check_kfunc_args() looks w/o this patch:
static int check_kfunc_args(...)
{
...
for (i = 0; i < nargs; i++) {
...
if (is_kfunc_arg_prog_aux(btf, &args[i])) {
... set env->insn_aux_data[insn_idx].arg_prog ...
}
if (is_kfunc_arg_ignore(btf, &args[i]) || is_kfunc_arg_implicit(meta, i))
continue;
...
}
return 0;
}
The `is_kfunc_arg_prog_aux(btf, &args[i])` might return false in case of bogus BTF.
In such a case `insn_aux_data[insn_idx].arg_prog` won't be set and no patching
would happen in check_kfunc_args(). The kfunc call would remain and the kfunc would
read whatever happens to be in the should-have-been patched register.
So this patch modifies check_kfunc_args() to error out if function is
marked with implicit args flag, but neither of known good implicit
args cases is present.
> It could be done immediately before patching, but at that point the verification is
> complete, right?..
Actually, yes. The following should be a legit fix as well (I think):
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index b27352d72b4f..6d6f4ccf2021 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1606,6 +1606,7 @@ enum bpf_reg_arg_type {
struct bpf_kfunc_desc {
struct btf_func_model func_model;
u32 func_id;
+ u32 flags;
s32 imm;
u16 offset;
unsigned long addr;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 02239c56801b..6e5f5a14a29c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2752,6 +2752,7 @@ int bpf_add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, u16 offset)
desc = &tab->descs[tab->nr_descs++];
desc->func_id = func_id;
+ desc->flags = kfunc.flags ? *kfunc.flags : 0;
desc->offset = offset;
desc->addr = addr;
desc->func_model = func_model;
@@ -19753,9 +19754,7 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
insn_buf[4] = BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1);
insn_buf[5] = BPF_ALU64_IMM(BPF_NEG, BPF_REG_0, 0);
*cnt = 6;
- }
-
- if (env->insn_aux_data[insn_idx].arg_prog) {
+ } else if (env->insn_aux_data[insn_idx].arg_prog) {
u32 regno = env->insn_aux_data[insn_idx].arg_prog;
struct bpf_insn ld_addrs[2] = { BPF_LD_IMM64(regno, (long)env->prog->aux) };
int idx = *cnt;
@@ -19764,6 +19763,10 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
insn_buf[idx++] = ld_addrs[1];
insn_buf[idx++] = *insn;
*cnt = idx;
+ } else if (desc->flags & KF_IMPLICIT_ARGS) {
+ verbose(env, "don't know how to patch kfunc call at instruction %d, possible BTF mismatch, kfunc is marked with KF_IMPLICIT_ARGS\n",
+ insn_idx);
+ return -EFAULT;
}
return 0;
}
> Other than that we'd have to somehow pass through from check_kfunc_args() to
> bpf_fixup_kfunc_call() information like "arg 1 of this kfunc can be patched to prog_aux" etc.
> We sort of do that already with the meta->arg_prog = true
>
> In a module one can define arbitrary kfuncs and add KF_IMPLICIT_ARGS to them, so
> proper hardening needs to be generic.
>
> I think this boils down to whether we want to error-check module kfuncs or not.
> Not sure what the verifier strategy is here, but my understanding is that in general
> a custom module can easily make kernel read garbage, and it's not a kernel's problem.
next prev parent reply other threads:[~2026-06-09 6:20 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-07 8:09 [PATCH] bpf: fix btf_types_are_same for cross-BTF type comparison chenyuan_fl
2026-04-07 8:58 ` Leon Hwang
2026-04-07 9:01 ` bot+bpf-ci
2026-04-07 11:19 ` Alan Maguire
2026-05-15 18:27 ` Ihor Solodrai
2026-06-01 6:46 ` [PATCH bpf v2 0/2] bpf: Fix kfunc implicit arg injection and add selftest chenyuan_fl
2026-06-01 6:46 ` [PATCH bpf v2 1/2] bpf: Fix kfunc implicit arg inject type detection to prevent invalid pointer deref chenyuan_fl
2026-06-01 7:42 ` bot+bpf-ci
2026-06-01 19:32 ` Eduard Zingerman
2026-06-02 8:58 ` [PATCH bpf v3 0/2] bpf: Fix kfunc implicit arg injection and add selftest chenyuan_fl
2026-06-02 8:58 ` [PATCH bpf v3 1/2] bpf: Fix kfunc implicit arg inject type detection to prevent invalid pointer deref chenyuan_fl
2026-06-02 9:44 ` bot+bpf-ci
2026-06-02 18:52 ` Ihor Solodrai
2026-06-04 9:14 ` chenyuan
2026-06-04 10:21 ` Alan Maguire
2026-06-09 0:46 ` Ihor Solodrai
2026-06-02 8:58 ` [PATCH bpf v3 2/2] selftests/bpf: Add regression test for kfunc implicit arg injection chenyuan_fl
2026-06-02 9:44 ` bot+bpf-ci
2026-06-02 9:38 ` [PATCH bpf v4 0/2] bpf: Fix kfunc implicit arg injection and add selftest chenyuan_fl
2026-06-02 9:38 ` [PATCH bpf v4 1/2] bpf: Fix kfunc implicit arg inject type detection to prevent invalid pointer deref chenyuan_fl
2026-06-02 10:42 ` bot+bpf-ci
2026-06-05 0:42 ` Eduard Zingerman
2026-06-02 9:38 ` [PATCH bpf v4 2/2] selftests/bpf: Add regression test for kfunc implicit arg injection chenyuan_fl
2026-06-02 10:27 ` bot+bpf-ci
2026-06-02 17:36 ` kernel test robot
2026-06-02 18:37 ` kernel test robot
2026-06-05 1:29 ` Eduard Zingerman
2026-06-08 14:26 ` [PATCH bpf v5 0/2] bpf: Fix kfunc implicit arg injection and add selftest chenyuan_fl
2026-06-08 14:26 ` [PATCH bpf v5 1/2] bpf: Fix kfunc implicit arg inject type detection to prevent invalid pointer deref chenyuan_fl
2026-06-08 14:52 ` bot+bpf-ci
2026-06-08 17:28 ` Eduard Zingerman
2026-06-09 0:54 ` Ihor Solodrai
2026-06-09 1:00 ` Eduard Zingerman
2026-06-09 1:18 ` Ihor Solodrai
2026-06-09 6:20 ` Eduard Zingerman [this message]
2026-06-08 14:26 ` [PATCH bpf v5 2/2] selftests/bpf: strengthen bpf_kfunc_implicit_arg to verify aux injection chenyuan_fl
2026-06-08 17:34 ` Eduard Zingerman
2026-06-09 0:59 ` Ihor Solodrai
2026-06-08 19:58 ` [PATCH bpf v5 0/2] bpf: Fix kfunc implicit arg injection and add selftest Alexei Starovoitov
2026-06-09 12:52 ` [PATCH bpf v6 " chenyuan_fl
2026-06-09 12:52 ` [PATCH bpf v6 1/2] bpf: Fix kfunc implicit arg inject type detection to prevent invalid pointer deref chenyuan_fl
2026-06-09 13:32 ` bot+bpf-ci
2026-06-09 16:39 ` Eduard Zingerman
2026-06-09 12:52 ` [PATCH bpf v6 2/2] selftests/bpf: strengthen bpf_kfunc_implicit_arg to verify aux injection chenyuan_fl
2026-06-01 17:12 ` [PATCH bpf v2 1/2] bpf: Fix kfunc implicit arg inject type detection to prevent invalid pointer deref Yonghong Song
2026-06-01 21:36 ` Eduard Zingerman
2026-06-01 6:46 ` [PATCH bpf v2 2/2] selftests/bpf: Add regression test for kfunc implicit arg injection with stale register chenyuan_fl
2026-06-01 17:17 ` Yonghong Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0d63a8d68bf0778d6e931ed27892a5bcb151295d.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bot+bpf-ci@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=chenyuan@kylinos.cn \
--cc=chenyuan_fl@163.com \
--cc=clm@meta.com \
--cc=daniel@iogearbox.net \
--cc=ihor.solodrai@linux.dev \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome