* [PATCH bpf v2 0/2] bpf: Fix tracing of kfuncs with implicit args
@ 2026-07-10 19:29 Ihor Solodrai
2026-07-10 19:29 ` [PATCH bpf v2 1/2] " Ihor Solodrai
2026-07-10 19:29 ` [PATCH bpf v2 2/2] selftests/bpf: Cover tracing implicit kfunc args Ihor Solodrai
0 siblings, 2 replies; 6+ messages in thread
From: Ihor Solodrai @ 2026-07-10 19:29 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Eduard Zingerman, Kumar Kartikeya Dwivedi
Cc: Tejun Heo, bpf, linux-kernel, kernel-team
Tejun reported an issue where a BPF program tracing a kfunc with
KF_IMPLICIT_ARGS can crash the kernel [1]. This is caused by a bug in
bpf_check_attach_target(): the btf_func_model for such a kfunc is
computed from a wrong BTF prototype. For more details see the commit
message of patch #1.
The second patch adds a selftest that can catch this situation.
The fix is a candidate for 7.1 backport.
[1] https://github.com/sched-ext/scx/issues/3687#issuecomment-4906694106
---
v1->v2:
* Take a module reference in btf_attach_func_proto() around the
btf_kfunc_accumulated_flags() call (sashiko)
v1: https://lore.kernel.org/bpf/20260710005902.2234832-1-ihor.solodrai@linux.dev/
---
Ihor Solodrai (1):
bpf: Fix tracing of kfuncs with implicit args
Kumar Kartikeya Dwivedi (1):
selftests/bpf: Cover tracing implicit kfunc args
include/linux/btf.h | 1 +
kernel/bpf/btf.c | 20 +++++
kernel/bpf/verifier.c | 64 +++++++++++----
.../prog_tests/kfunc_implicit_args_tracing.c | 36 +++++++++
.../bpf/progs/kfunc_implicit_args_tracing.c | 77 +++++++++++++++++++
5 files changed, 185 insertions(+), 13 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kfunc_implicit_args_tracing.c
create mode 100644 tools/testing/selftests/bpf/progs/kfunc_implicit_args_tracing.c
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args 2026-07-10 19:29 [PATCH bpf v2 0/2] bpf: Fix tracing of kfuncs with implicit args Ihor Solodrai @ 2026-07-10 19:29 ` Ihor Solodrai 2026-07-11 10:43 ` Eduard Zingerman 2026-07-10 19:29 ` [PATCH bpf v2 2/2] selftests/bpf: Cover tracing implicit kfunc args Ihor Solodrai 1 sibling, 1 reply; 6+ messages in thread From: Ihor Solodrai @ 2026-07-10 19:29 UTC (permalink / raw) To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Eduard Zingerman, Kumar Kartikeya Dwivedi Cc: Tejun Heo, bpf, linux-kernel, kernel-team A kfunc marked with KF_IMPLICIT_ARGS flag takes implicit arguments (such as bpf_prog_aux) that the verifier injects at load time. resolve_btfids strips those from the kfunc's BTF-visible prototype and keeps the real kernel ABI in a counterpart _impl prototype [1]. fentry/fexit/fmod_ret/fsession programs may attach to the BPF kernel functions, including those with implicit args. However bpf_check_attach_target() and bpf_check_attach_btf_id_multi() extract the struct btf_func_model from the wrong BTF prototype of the kfunc. The btf_func_model is later read to construct the trampoline, which then causes the injected implicit argument to be clobbered and the kfunc dereferencing garbage. Add btf_attach_func_proto() to resolve the real ABI prototype of the kfunc the way the call site does: by looking up the _impl prototype for a KF_IMPLICIT_ARGS kfunc. Use it at both attach-target model construction sites. To enable this, make two supporting changes: * pass bpf_verifier_log instead of bpf_verifier_env to find_kfunc_impl_proto(), so it can be reused from the attach path * introduce btf_kfunc_accumulated_flags() helper to read a kfunc's flags across all hooks, because a program attaching to a kfunc is not in the kfunc's call-set btf_attach_func_proto() reads the kfunc's flags from the target's kfunc_set_tab via btf_kfunc_accumulated_flags(). For a module BTF that table is stable only after the module is live, so take a module reference around the read, mirroring how the kfunc call path gates the same lookup with btf_try_get_module(). The remaining call sites of btf_distill_func_proto() are safe as is. The BPF_TRACE_ITER case distills a registered iterator's prototype, and bpf_struct_ops_desc_init() distills the function-pointer members of a struct_ops type. Neither is a kfunc, and so can't have implicit arguments. [1] https://lore.kernel.org/all/20260120222638.3976562-1-ihor.solodrai@linux.dev/ Fixes: 64e1360524b9 ("bpf: Verifier support for KF_IMPLICIT_ARGS") Reported-by: Tejun Heo <tj@kernel.org> Link: https://github.com/sched-ext/scx/issues/3687#issuecomment-4906694106 Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- include/linux/btf.h | 1 + kernel/bpf/btf.c | 20 ++++++++++++++ kernel/bpf/verifier.c | 64 ++++++++++++++++++++++++++++++++++--------- 3 files changed, 72 insertions(+), 13 deletions(-) diff --git a/include/linux/btf.h b/include/linux/btf.h index 240401d9b25b..c52245ae0df5 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -578,6 +578,7 @@ const char *btf_str_by_offset(const struct btf *btf, u32 offset); struct btf *btf_parse_vmlinux(void); struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog); u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_prog *prog); +u32 btf_kfunc_accumulated_flags(const struct btf *btf, u32 kfunc_btf_id); bool btf_kfunc_is_allowed(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_prog *prog); u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_prog *prog); diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 64572f85edc8..20aeca6b4f95 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -9114,6 +9114,26 @@ u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_p return btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id); } +/* + * Return the union of a kfunc's flags across all hooks. + * Unlike btf_kfunc_flags(), not restricted to a calling + * program's hook. Used when attaching to a kfunc for tracing. + */ +u32 btf_kfunc_accumulated_flags(const struct btf *btf, u32 kfunc_btf_id) +{ + enum btf_kfunc_hook hook; + u32 *hook_flags; + u32 flags = 0; + + for (hook = 0; hook < BTF_KFUNC_HOOK_MAX; hook++) { + hook_flags = btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id); + if (hook_flags) + flags |= *hook_flags; + } + + return flags; +} + u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_prog *prog) { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 6515d4d3c003..2f56ab8d6b58 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2584,24 +2584,24 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset) #define KF_IMPL_SUFFIX "_impl" -static const struct btf_type *find_kfunc_impl_proto(struct bpf_verifier_env *env, - struct btf *btf, - const char *func_name) +static const struct btf_type * +find_kfunc_impl_proto(struct bpf_verifier_log *log, struct btf *btf, const char *func_name) { - char *buf = env->tmp_str_buf; const struct btf_type *func; + char buf[KSYM_NAME_LEN]; s32 impl_id; int len; - len = snprintf(buf, TMP_STR_BUF_LEN, "%s%s", func_name, KF_IMPL_SUFFIX); - if (len < 0 || len >= TMP_STR_BUF_LEN) { - verbose(env, "function name %s%s is too long\n", func_name, KF_IMPL_SUFFIX); + len = snprintf(buf, sizeof(buf), "%s%s", func_name, KF_IMPL_SUFFIX); + if (len < 0 || len >= sizeof(buf)) { + bpf_log(log, "function name %s%s is too long\n", + func_name, KF_IMPL_SUFFIX); return NULL; } impl_id = btf_find_by_name_kind(btf, buf, BTF_KIND_FUNC); if (impl_id <= 0) { - verbose(env, "cannot find function %s in BTF\n", buf); + bpf_log(log, "cannot find function %s in BTF\n", buf); return NULL; } @@ -2653,7 +2653,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env, * can be found through the counterpart _impl kfunc. */ if (kfunc_flags && (*kfunc_flags & KF_IMPLICIT_ARGS)) - func_proto = find_kfunc_impl_proto(env, btf, func_name); + func_proto = find_kfunc_impl_proto(&env->log, btf, func_name); else func_proto = btf_type_by_id(btf, func->type); @@ -18873,6 +18873,44 @@ static int btf_id_allow_sleepable(u32 btf_id, unsigned long addr, const struct b return -EINVAL; } +/* + * Resolve the prototype describing a trace target's real ABI. A + * KF_IMPLICIT_ARGS kfunc has its injected args stripped from the public + * prototype, so use the _impl prototype; other targets use their own. + */ +static const struct btf_type * +btf_attach_func_proto(struct bpf_verifier_log *log, struct btf *btf, u32 func_id) +{ + const struct btf_type *func; + struct module *mod = NULL; + const char *name; + u32 kfunc_flags; + + func = btf_type_by_id(btf, func_id); + if (!func || !btf_type_is_func(func)) + return NULL; + + /* + * btf_kfunc_accumulated_flags() reads kfunc_set_tab, which for a + * module is stable only once it is live; hold a module ref across + * the read to exclude a concurrent module load. + */ + if (btf_is_module(btf)) { + mod = btf_try_get_module(btf); + if (!mod) + return NULL; + } + kfunc_flags = btf_kfunc_accumulated_flags(btf, func_id); + module_put(mod); + + if (kfunc_flags & KF_IMPLICIT_ARGS) { + name = btf_name_by_offset(btf, func->name_off); + return find_kfunc_impl_proto(log, btf, name); + } + + return btf_type_by_id(btf, func->type); +} + int bpf_check_attach_target(struct bpf_verifier_log *log, const struct bpf_prog *prog, const struct bpf_prog *tgt_prog, @@ -19121,8 +19159,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, if (prog_extension && btf_check_type_match(log, prog, btf, t)) return -EINVAL; - t = btf_type_by_id(btf, t->type); - if (!btf_type_is_func_proto(t)) + t = btf_attach_func_proto(log, btf, btf_id); + if (!t || !btf_type_is_func_proto(t)) return -EINVAL; if ((prog->aux->saved_dst_prog_type || prog->aux->saved_dst_attach_type) && @@ -19407,8 +19445,8 @@ int bpf_check_attach_btf_id_multi(struct btf *btf, struct bpf_prog *prog, u32 bt return -EINVAL; if (!btf_type_is_func(t)) return -EINVAL; - t = btf_type_by_id(btf, t->type); - if (!btf_type_is_func_proto(t)) + t = btf_attach_func_proto(NULL, btf, btf_id); + if (!t || !btf_type_is_func_proto(t)) return -EINVAL; err = btf_distill_func_proto(NULL, btf, t, tname, &tgt_info->fmodel); if (err < 0) -- 2.55.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args 2026-07-10 19:29 ` [PATCH bpf v2 1/2] " Ihor Solodrai @ 2026-07-11 10:43 ` Eduard Zingerman 2026-07-11 17:57 ` Ihor Solodrai 0 siblings, 1 reply; 6+ messages in thread From: Eduard Zingerman @ 2026-07-11 10:43 UTC (permalink / raw) To: Ihor Solodrai, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kumar Kartikeya Dwivedi Cc: Tejun Heo, bpf, linux-kernel, kernel-team On Fri, 2026-07-10 at 12:29 -0700, Ihor Solodrai wrote: [...] > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 64572f85edc8..20aeca6b4f95 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -9114,6 +9114,26 @@ u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_p > return btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id); > } > > +/* > + * Return the union of a kfunc's flags across all hooks. > + * Unlike btf_kfunc_flags(), not restricted to a calling > + * program's hook. Used when attaching to a kfunc for tracing. > + */ > +u32 btf_kfunc_accumulated_flags(const struct btf *btf, u32 kfunc_btf_id) > +{ > + enum btf_kfunc_hook hook; > + u32 *hook_flags; > + u32 flags = 0; > + > + for (hook = 0; hook < BTF_KFUNC_HOOK_MAX; hook++) { > + hook_flags = btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id); > + if (hook_flags) > + flags |= *hook_flags; > + } > + > + return flags; > +} This. Is. So. Ugly. You guys should reflect a bit more on the llm output. The fact that we currently have a technical possibility to have the same kfunc in different sets with different flags seem to be accidental. Is there ever a valid scenario for this? Grepping kernel code all I can find are these kfuncs from drivers/hid/bpf/hid_bpf_dispatch.c: BTF_KFUNCS_START(hid_bpf_kfunc_ids) ... BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE) BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE | KF_SLEEPABLE) BTF_ID_FLAGS(func, hid_bpf_hw_request, KF_SLEEPABLE) BTF_ID_FLAGS(func, hid_bpf_hw_output_report, KF_SLEEPABLE) BTF_ID_FLAGS(func, hid_bpf_input_report, KF_SLEEPABLE) BTF_ID_FLAGS(func, hid_bpf_try_input_report) BTF_KFUNCS_END(hid_bpf_kfunc_ids) ... /* for syscall HID-BPF */ BTF_KFUNCS_START(hid_bpf_syscall_kfunc_ids) BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL) BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE) BTF_ID_FLAGS(func, hid_bpf_hw_request) BTF_ID_FLAGS(func, hid_bpf_hw_output_report) BTF_ID_FLAGS(func, hid_bpf_input_report) BTF_KFUNCS_END(hid_bpf_syscall_kfunc_ids) ... register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &hid_bpf_syscall_kfunc_set); But this appears to be a misuse: (a) syscall programs are always sleepable (b) e.g. hid_bpf_allocate_context() calls kzalloc_obj() which can sleep, as far as I understand. Meaning that hid_bpf_allocate_context() should always be marked with KF_SLEEPABLE. Am I confused? If I am not confused, I think that resolve_btfids() has to verify that kfunc flags are always the same across multiple sets. > + > u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id, > const struct bpf_prog *prog) > { > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 6515d4d3c003..2f56ab8d6b58 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -2584,24 +2584,24 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset) > > #define KF_IMPL_SUFFIX "_impl" > > -static const struct btf_type *find_kfunc_impl_proto(struct bpf_verifier_env *env, > - struct btf *btf, > - const char *func_name) > +static const struct btf_type * > +find_kfunc_impl_proto(struct bpf_verifier_log *log, struct btf *btf, const char *func_name) Nit: no need to reformat the declaration. > { > - char *buf = env->tmp_str_buf; > const struct btf_type *func; > + char buf[KSYM_NAME_LEN]; We attempt to avoid large allocations on stack, that's why tmp_str_buf was used here in a first place. This is the second time I see llm doing this, maybe adjust your agents.md or something. If tmp_str_buf is not big enough, that can be adjusted. But I don't think it's necessary in this particular case. > s32 impl_id; > int len; > > - len = snprintf(buf, TMP_STR_BUF_LEN, "%s%s", func_name, KF_IMPL_SUFFIX); > - if (len < 0 || len >= TMP_STR_BUF_LEN) { > - verbose(env, "function name %s%s is too long\n", func_name, KF_IMPL_SUFFIX); > + len = snprintf(buf, sizeof(buf), "%s%s", func_name, KF_IMPL_SUFFIX); > + if (len < 0 || len >= sizeof(buf)) { > + bpf_log(log, "function name %s%s is too long\n", > + func_name, KF_IMPL_SUFFIX); > return NULL; > } > > impl_id = btf_find_by_name_kind(btf, buf, BTF_KIND_FUNC); > if (impl_id <= 0) { > - verbose(env, "cannot find function %s in BTF\n", buf); > + bpf_log(log, "cannot find function %s in BTF\n", buf); > return NULL; > } > > @@ -2653,7 +2653,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env, > * can be found through the counterpart _impl kfunc. > */ > if (kfunc_flags && (*kfunc_flags & KF_IMPLICIT_ARGS)) > - func_proto = find_kfunc_impl_proto(env, btf, func_name); > + func_proto = find_kfunc_impl_proto(&env->log, btf, func_name); > else > func_proto = btf_type_by_id(btf, func->type); > > @@ -18873,6 +18873,44 @@ static int btf_id_allow_sleepable(u32 btf_id, unsigned long addr, const struct b > return -EINVAL; > } > > +/* > + * Resolve the prototype describing a trace target's real ABI. A > + * KF_IMPLICIT_ARGS kfunc has its injected args stripped from the public > + * prototype, so use the _impl prototype; other targets use their own. > + */ > +static const struct btf_type * > +btf_attach_func_proto(struct bpf_verifier_log *log, struct btf *btf, u32 func_id) > +{ > + const struct btf_type *func; > + struct module *mod = NULL; > + const char *name; > + u32 kfunc_flags; > + > + func = btf_type_by_id(btf, func_id); > + if (!func || !btf_type_is_func(func)) > + return NULL; > + > + /* > + * btf_kfunc_accumulated_flags() reads kfunc_set_tab, which for a > + * module is stable only once it is live; hold a module ref across > + * the read to exclude a concurrent module load. > + */ > + if (btf_is_module(btf)) { So, this function is supplied with a module BTF, and the argument is that we can have an alive BTF reference but not to hold a live module reference? References to kfunc's modules are stored in the bpf_prog_aux->kfunc_btf_tab, and the following code from core.c suggests that module references would be present for as long as the program using BTF from the module is present: static void bpf_prog_free_deferred(struct work_struct *work) { ... bpf_free_kfunc_btf_tab(aux->kfunc_btf_tab); ... } ? > + mod = btf_try_get_module(btf); > + if (!mod) > + return NULL; > + } > + kfunc_flags = btf_kfunc_accumulated_flags(btf, func_id); > + module_put(mod); > + > + if (kfunc_flags & KF_IMPLICIT_ARGS) { > + name = btf_name_by_offset(btf, func->name_off); > + return find_kfunc_impl_proto(log, btf, name); > + } > + > + return btf_type_by_id(btf, func->type); > +} > + > int bpf_check_attach_target(struct bpf_verifier_log *log, > const struct bpf_prog *prog, > const struct bpf_prog *tgt_prog, > @@ -19121,8 +19159,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, > if (prog_extension && > btf_check_type_match(log, prog, btf, t)) > return -EINVAL; > - t = btf_type_by_id(btf, t->type); > - if (!btf_type_is_func_proto(t)) > + t = btf_attach_func_proto(log, btf, btf_id); > + if (!t || !btf_type_is_func_proto(t)) This part seems fine. > return -EINVAL; > > if ((prog->aux->saved_dst_prog_type || prog->aux->saved_dst_attach_type) && > @@ -19407,8 +19445,8 @@ int bpf_check_attach_btf_id_multi(struct btf *btf, struct bpf_prog *prog, u32 bt > return -EINVAL; > if (!btf_type_is_func(t)) > return -EINVAL; > - t = btf_type_by_id(btf, t->type); > - if (!btf_type_is_func_proto(t)) > + t = btf_attach_func_proto(NULL, btf, btf_id); > + if (!t || !btf_type_is_func_proto(t)) > return -EINVAL; > err = btf_distill_func_proto(NULL, btf, t, tname, &tgt_info->fmodel); > if (err < 0) > -- > 2.55.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args 2026-07-11 10:43 ` Eduard Zingerman @ 2026-07-11 17:57 ` Ihor Solodrai 2026-07-11 23:51 ` Eduard Zingerman 0 siblings, 1 reply; 6+ messages in thread From: Ihor Solodrai @ 2026-07-11 17:57 UTC (permalink / raw) To: Eduard Zingerman, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kumar Kartikeya Dwivedi Cc: Tejun Heo, bpf, linux-kernel, kernel-team On 2026-07-11 3:43 a.m., Eduard Zingerman wrote: > On Fri, 2026-07-10 at 12:29 -0700, Ihor Solodrai wrote: > > [...] > >> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c >> index 64572f85edc8..20aeca6b4f95 100644 >> --- a/kernel/bpf/btf.c >> +++ b/kernel/bpf/btf.c >> @@ -9114,6 +9114,26 @@ u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_p >> return btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id); >> } >> >> +/* >> + * Return the union of a kfunc's flags across all hooks. >> + * Unlike btf_kfunc_flags(), not restricted to a calling >> + * program's hook. Used when attaching to a kfunc for tracing. >> + */ >> +u32 btf_kfunc_accumulated_flags(const struct btf *btf, u32 kfunc_btf_id) >> +{ >> + enum btf_kfunc_hook hook; >> + u32 *hook_flags; >> + u32 flags = 0; >> + >> + for (hook = 0; hook < BTF_KFUNC_HOOK_MAX; hook++) { >> + hook_flags = btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id); >> + if (hook_flags) >> + flags |= *hook_flags; >> + } >> + >> + return flags; >> +} > > This. Is. So. Ugly. You guys should reflect a bit more on the llm output. Hi Eduard, thank you for the review. I think it's healthy to have a clanker hater bias on the list, given the circumstances. However not every slop is a decision made by a LLM, it's often humans. > > The fact that we currently have a technical possibility to have the same > kfunc in different sets with different flags seem to be accidental. > Is there ever a valid scenario for this? I don't know about "valid", but in fact there are kfuncs that currently have inconsistent flags. For example KF_SLEEPABLE can be on or off depending on the kfunc set: https://lore.kernel.org/bpf/ah8-6CNIHPCJxAOM@krava/ > Grepping kernel code all I can find are these kfuncs from > drivers/hid/bpf/hid_bpf_dispatch.c: > > BTF_KFUNCS_START(hid_bpf_kfunc_ids) > ... > BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE) > BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE | KF_SLEEPABLE) > BTF_ID_FLAGS(func, hid_bpf_hw_request, KF_SLEEPABLE) > BTF_ID_FLAGS(func, hid_bpf_hw_output_report, KF_SLEEPABLE) > BTF_ID_FLAGS(func, hid_bpf_input_report, KF_SLEEPABLE) > BTF_ID_FLAGS(func, hid_bpf_try_input_report) > BTF_KFUNCS_END(hid_bpf_kfunc_ids) > ... > /* for syscall HID-BPF */ > BTF_KFUNCS_START(hid_bpf_syscall_kfunc_ids) > BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL) > BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE) > BTF_ID_FLAGS(func, hid_bpf_hw_request) > BTF_ID_FLAGS(func, hid_bpf_hw_output_report) > BTF_ID_FLAGS(func, hid_bpf_input_report) > BTF_KFUNCS_END(hid_bpf_syscall_kfunc_ids) > ... > register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &hid_bpf_syscall_kfunc_set); > > But this appears to be a misuse: > (a) syscall programs are always sleepable > (b) e.g. hid_bpf_allocate_context() calls kzalloc_obj() which can sleep, > as far as I understand. Meaning that hid_bpf_allocate_context() > should always be marked with KF_SLEEPABLE. > > Am I confused? > > If I am not confused, I think that resolve_btfids() has to verify that > kfunc flags are always the same across multiple sets. Some form of build-time enforcement will be implemented as part of resolve_btfids() BTF handling, see the v1 here: https://lore.kernel.org/bpf/20260601221805.821394-1-ihor.solodrai@linux.dev/ *If* you're right about no valid use-case for incosistent kfunc flags, then long-term I agree: build-time enforcement of consistency, and then the acummulation can be replaced by a search (find first) here. But this has to be confirmed somehow (by thoroughly inspecting all current inconsistencies and flags?..) However if there is even one valid use-case, then we are in trouble, because then depending on the flag semantics it may or may not make sense for it to be consistent accross kfunc sets. And it's not even clear what a good solution to that might look like: separate groups of flags? consistency enforcement for some flags, but not others (this is literally what Andrii suggested in resolve_btfids thread)? Accumulating flags across the sets is an acceptable workaround for this fix IMO. We can be more specific and *only* check for the KF_IMPLICIT_ARGS for the purposes of this fix though, but we still have to walk all hooks and OR. I don't think it's reasonable to wait for the comprehensive kfunc flags analysis and resolve_btfids series landing before fixing the garbage dereference bug. > >> + >> u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id, >> const struct bpf_prog *prog) >> { >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 6515d4d3c003..2f56ab8d6b58 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -2584,24 +2584,24 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset) >> >> #define KF_IMPL_SUFFIX "_impl" >> >> -static const struct btf_type *find_kfunc_impl_proto(struct bpf_verifier_env *env, >> - struct btf *btf, >> - const char *func_name) >> +static const struct btf_type * >> +find_kfunc_impl_proto(struct bpf_verifier_log *log, struct btf *btf, const char *func_name) > > Nit: no need to reformat the declaration. > >> { >> - char *buf = env->tmp_str_buf; >> const struct btf_type *func; >> + char buf[KSYM_NAME_LEN]; > > We attempt to avoid large allocations on stack, that's why tmp_str_buf > was used here in a first place. This is the second time I see llm doing this, > maybe adjust your agents.md or something. If tmp_str_buf is not big enough, > that can be adjusted. But I don't think it's necessary in this particular case. This is not just a llm artifact, the new callers in the attach path don't have env available, only the log. Do you suggest refactoring bpf_check_attach_target() and path through the env now just to avoid allocating this buffer? I aimed to keep the fix patch minimal, but I see this is going to be a challenge. > >> s32 impl_id; >> int len; >> >> - len = snprintf(buf, TMP_STR_BUF_LEN, "%s%s", func_name, KF_IMPL_SUFFIX); >> - if (len < 0 || len >= TMP_STR_BUF_LEN) { >> - verbose(env, "function name %s%s is too long\n", func_name, KF_IMPL_SUFFIX); >> + len = snprintf(buf, sizeof(buf), "%s%s", func_name, KF_IMPL_SUFFIX); >> + if (len < 0 || len >= sizeof(buf)) { >> + bpf_log(log, "function name %s%s is too long\n", >> + func_name, KF_IMPL_SUFFIX); >> return NULL; >> } >> >> impl_id = btf_find_by_name_kind(btf, buf, BTF_KIND_FUNC); >> if (impl_id <= 0) { >> - verbose(env, "cannot find function %s in BTF\n", buf); >> + bpf_log(log, "cannot find function %s in BTF\n", buf); >> return NULL; >> } >> >> @@ -2653,7 +2653,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env, >> * can be found through the counterpart _impl kfunc. >> */ >> if (kfunc_flags && (*kfunc_flags & KF_IMPLICIT_ARGS)) >> - func_proto = find_kfunc_impl_proto(env, btf, func_name); >> + func_proto = find_kfunc_impl_proto(&env->log, btf, func_name); >> else >> func_proto = btf_type_by_id(btf, func->type); >> >> @@ -18873,6 +18873,44 @@ static int btf_id_allow_sleepable(u32 btf_id, unsigned long addr, const struct b >> return -EINVAL; >> } >> >> +/* >> + * Resolve the prototype describing a trace target's real ABI. A >> + * KF_IMPLICIT_ARGS kfunc has its injected args stripped from the public >> + * prototype, so use the _impl prototype; other targets use their own. >> + */ >> +static const struct btf_type * >> +btf_attach_func_proto(struct bpf_verifier_log *log, struct btf *btf, u32 func_id) >> +{ >> + const struct btf_type *func; >> + struct module *mod = NULL; >> + const char *name; >> + u32 kfunc_flags; >> + >> + func = btf_type_by_id(btf, func_id); >> + if (!func || !btf_type_is_func(func)) >> + return NULL; >> + >> + /* >> + * btf_kfunc_accumulated_flags() reads kfunc_set_tab, which for a >> + * module is stable only once it is live; hold a module ref across >> + * the read to exclude a concurrent module load. >> + */ >> + if (btf_is_module(btf)) { > > So, this function is supplied with a module BTF, > and the argument is that we can have an alive BTF reference > but not to hold a live module reference? We have a convention (I don't know the history behind it) to always btf_try_get_module() before reading associated kfunc sets, see a comment in btf.c: /* Caution: * Reference to the module (obtained using btf_try_get_module) corresponding to * the struct btf *MUST* be held when calling this function from verifier * context. This is usually true as we stash references in prog's kfunc_btf_tab; * keeping the reference for the duration of the call provides the necessary * protection for looking up a well-formed btf->kfunc_set_tab. */ I don't think it's worth the risk violating this convention within the fix, even though you may be right about it being unnecessary in principle. > > References to kfunc's modules are stored in the > bpf_prog_aux->kfunc_btf_tab, and the following code from core.c > suggests that module references would be present for as long > as the program using BTF from the module is present: > > static void bpf_prog_free_deferred(struct work_struct *work) > { > ... > bpf_free_kfunc_btf_tab(aux->kfunc_btf_tab); > ... > } > > ? > >> + mod = btf_try_get_module(btf); >> + if (!mod) >> + return NULL; >> + } >> + kfunc_flags = btf_kfunc_accumulated_flags(btf, func_id); >> + module_put(mod); >> + >> + if (kfunc_flags & KF_IMPLICIT_ARGS) { >> + name = btf_name_by_offset(btf, func->name_off); >> + return find_kfunc_impl_proto(log, btf, name); >> + } >> + >> + return btf_type_by_id(btf, func->type); >> +} >> + >> int bpf_check_attach_target(struct bpf_verifier_log *log, >> const struct bpf_prog *prog, >> const struct bpf_prog *tgt_prog, >> @@ -19121,8 +19159,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, >> if (prog_extension && >> btf_check_type_match(log, prog, btf, t)) >> return -EINVAL; >> - t = btf_type_by_id(btf, t->type); >> - if (!btf_type_is_func_proto(t)) >> + t = btf_attach_func_proto(log, btf, btf_id); >> + if (!t || !btf_type_is_func_proto(t)) > > This part seems fine. > >> return -EINVAL; >> >> if ((prog->aux->saved_dst_prog_type || prog->aux->saved_dst_attach_type) && >> @@ -19407,8 +19445,8 @@ int bpf_check_attach_btf_id_multi(struct btf *btf, struct bpf_prog *prog, u32 bt >> return -EINVAL; >> if (!btf_type_is_func(t)) >> return -EINVAL; >> - t = btf_type_by_id(btf, t->type); >> - if (!btf_type_is_func_proto(t)) >> + t = btf_attach_func_proto(NULL, btf, btf_id); >> + if (!t || !btf_type_is_func_proto(t)) >> return -EINVAL; >> err = btf_distill_func_proto(NULL, btf, t, tname, &tgt_info->fmodel); >> if (err < 0) >> -- >> 2.55.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args 2026-07-11 17:57 ` Ihor Solodrai @ 2026-07-11 23:51 ` Eduard Zingerman 0 siblings, 0 replies; 6+ messages in thread From: Eduard Zingerman @ 2026-07-11 23:51 UTC (permalink / raw) To: Ihor Solodrai, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kumar Kartikeya Dwivedi Cc: Tejun Heo, bpf, linux-kernel, kernel-team On Sat, 2026-07-11 at 10:57 -0700, Ihor Solodrai wrote: > On 2026-07-11 3:43 a.m., Eduard Zingerman wrote: > > On Fri, 2026-07-10 at 12:29 -0700, Ihor Solodrai wrote: > > > > [...] > > > > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > > > index 64572f85edc8..20aeca6b4f95 100644 > > > --- a/kernel/bpf/btf.c > > > +++ b/kernel/bpf/btf.c > > > @@ -9114,6 +9114,26 @@ u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_p > > > return btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id); > > > } > > > > > > +/* > > > + * Return the union of a kfunc's flags across all hooks. > > > + * Unlike btf_kfunc_flags(), not restricted to a calling > > > + * program's hook. Used when attaching to a kfunc for tracing. > > > + */ > > > +u32 btf_kfunc_accumulated_flags(const struct btf *btf, u32 kfunc_btf_id) > > > +{ > > > + enum btf_kfunc_hook hook; > > > + u32 *hook_flags; > > > + u32 flags = 0; > > > + > > > + for (hook = 0; hook < BTF_KFUNC_HOOK_MAX; hook++) { > > > + hook_flags = btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id); > > > + if (hook_flags) > > > + flags |= *hook_flags; > > > + } > > > + > > > + return flags; > > > +} > > > > This. Is. So. Ugly. You guys should reflect a bit more on the llm output. > > Hi Eduard, thank you for the review. > > I think it's healthy to have a clanker hater bias on the list, > given the circumstances. However not every slop is a decision made > by a LLM, it's often humans. > > > > > The fact that we currently have a technical possibility to have the same > > kfunc in different sets with different flags seem to be accidental. > > Is there ever a valid scenario for this? > > I don't know about "valid", but in fact there are kfuncs that > currently have inconsistent flags. > > For example KF_SLEEPABLE can be on or off depending on the kfunc set: > https://lore.kernel.org/bpf/ah8-6CNIHPCJxAOM@krava/ > > > > Grepping kernel code all I can find are these kfuncs from > > drivers/hid/bpf/hid_bpf_dispatch.c: > > > > BTF_KFUNCS_START(hid_bpf_kfunc_ids) > > ... > > BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE) > > BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE | KF_SLEEPABLE) > > BTF_ID_FLAGS(func, hid_bpf_hw_request, KF_SLEEPABLE) > > BTF_ID_FLAGS(func, hid_bpf_hw_output_report, KF_SLEEPABLE) > > BTF_ID_FLAGS(func, hid_bpf_input_report, KF_SLEEPABLE) > > BTF_ID_FLAGS(func, hid_bpf_try_input_report) > > BTF_KFUNCS_END(hid_bpf_kfunc_ids) > > ... > > /* for syscall HID-BPF */ > > BTF_KFUNCS_START(hid_bpf_syscall_kfunc_ids) > > BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL) > > BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE) > > BTF_ID_FLAGS(func, hid_bpf_hw_request) > > BTF_ID_FLAGS(func, hid_bpf_hw_output_report) > > BTF_ID_FLAGS(func, hid_bpf_input_report) > > BTF_KFUNCS_END(hid_bpf_syscall_kfunc_ids) > > ... > > register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &hid_bpf_syscall_kfunc_set); > > > > But this appears to be a misuse: > > (a) syscall programs are always sleepable > > (b) e.g. hid_bpf_allocate_context() calls kzalloc_obj() which can sleep, > > as far as I understand. Meaning that hid_bpf_allocate_context() > > should always be marked with KF_SLEEPABLE. > > > > Am I confused? > > > > If I am not confused, I think that resolve_btfids() has to verify that > > kfunc flags are always the same across multiple sets. > > Some form of build-time enforcement will be implemented as part of > resolve_btfids() BTF handling, see the v1 here: > https://lore.kernel.org/bpf/20260601221805.821394-1-ihor.solodrai@linux.dev/ > > *If* you're right about no valid use-case for incosistent kfunc flags, > then long-term I agree: build-time enforcement of consistency, and > then the acummulation can be replaced by a search (find first) here. > > But this has to be confirmed somehow (by thoroughly inspecting all > current inconsistencies and flags?..) > > However if there is even one valid use-case, then we are in trouble, > because then depending on the flag semantics it may or may not make > sense for it to be consistent accross kfunc sets. And it's not even > clear what a good solution to that might look like: separate groups of > flags? consistency enforcement for some flags, but not others (this is > literally what Andrii suggested in resolve_btfids thread)? > > Accumulating flags across the sets is an acceptable workaround for > this fix IMO. We can be more specific and *only* check for the > KF_IMPLICIT_ARGS for the purposes of this fix though, but we still > have to walk all hooks and OR. And what would that mean for the same function to have KF_IMPLICIT_ARGS in one set and not to have it in another? Given that actual function address is resolved to the exact same function. Such accumulator would only proliferate already confusing behaviour. I don't think that the analysis of existing cases would take longer than 1-2h. > I don't think it's reasonable to wait for the comprehensive kfunc flags > analysis and resolve_btfids series landing before fixing the garbage > dereference bug. > > > > > > + > > > u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id, > > > const struct bpf_prog *prog) > > > { > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > > index 6515d4d3c003..2f56ab8d6b58 100644 > > > --- a/kernel/bpf/verifier.c > > > +++ b/kernel/bpf/verifier.c > > > @@ -2584,24 +2584,24 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset) > > > > > > #define KF_IMPL_SUFFIX "_impl" > > > > > > -static const struct btf_type *find_kfunc_impl_proto(struct bpf_verifier_env *env, > > > - struct btf *btf, > > > - const char *func_name) > > > +static const struct btf_type * > > > +find_kfunc_impl_proto(struct bpf_verifier_log *log, struct btf *btf, const char *func_name) > > > > Nit: no need to reformat the declaration. > > > > > { > > > - char *buf = env->tmp_str_buf; > > > const struct btf_type *func; > > > + char buf[KSYM_NAME_LEN]; > > > > We attempt to avoid large allocations on stack, that's why tmp_str_buf > > was used here in a first place. This is the second time I see llm doing this, > > maybe adjust your agents.md or something. If tmp_str_buf is not big enough, > > that can be adjusted. But I don't think it's necessary in this particular case. > > This is not just a llm artifact, the new callers in the attach path > don't have env available, only the log. Do you suggest refactoring > bpf_check_attach_target() and path through the env now just to avoid > allocating this buffer? > > I aimed to keep the fix patch minimal, but I see this is going to be a > challenge. My bad, sorry. > > > > > s32 impl_id; > > > int len; > > > > > > - len = snprintf(buf, TMP_STR_BUF_LEN, "%s%s", func_name, KF_IMPL_SUFFIX); > > > - if (len < 0 || len >= TMP_STR_BUF_LEN) { > > > - verbose(env, "function name %s%s is too long\n", func_name, KF_IMPL_SUFFIX); > > > + len = snprintf(buf, sizeof(buf), "%s%s", func_name, KF_IMPL_SUFFIX); > > > + if (len < 0 || len >= sizeof(buf)) { > > > + bpf_log(log, "function name %s%s is too long\n", > > > + func_name, KF_IMPL_SUFFIX); > > > return NULL; > > > } > > > > > > impl_id = btf_find_by_name_kind(btf, buf, BTF_KIND_FUNC); > > > if (impl_id <= 0) { > > > - verbose(env, "cannot find function %s in BTF\n", buf); > > > + bpf_log(log, "cannot find function %s in BTF\n", buf); > > > return NULL; > > > } > > > > > > @@ -2653,7 +2653,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env, > > > * can be found through the counterpart _impl kfunc. > > > */ > > > if (kfunc_flags && (*kfunc_flags & KF_IMPLICIT_ARGS)) > > > - func_proto = find_kfunc_impl_proto(env, btf, func_name); > > > + func_proto = find_kfunc_impl_proto(&env->log, btf, func_name); > > > else > > > func_proto = btf_type_by_id(btf, func->type); > > > > > > @@ -18873,6 +18873,44 @@ static int btf_id_allow_sleepable(u32 btf_id, unsigned long addr, const struct b > > > return -EINVAL; > > > } > > > > > > +/* > > > + * Resolve the prototype describing a trace target's real ABI. A > > > + * KF_IMPLICIT_ARGS kfunc has its injected args stripped from the public > > > + * prototype, so use the _impl prototype; other targets use their own. > > > + */ > > > +static const struct btf_type * > > > +btf_attach_func_proto(struct bpf_verifier_log *log, struct btf *btf, u32 func_id) > > > +{ > > > + const struct btf_type *func; > > > + struct module *mod = NULL; > > > + const char *name; > > > + u32 kfunc_flags; > > > + > > > + func = btf_type_by_id(btf, func_id); > > > + if (!func || !btf_type_is_func(func)) > > > + return NULL; > > > + > > > + /* > > > + * btf_kfunc_accumulated_flags() reads kfunc_set_tab, which for a > > > + * module is stable only once it is live; hold a module ref across > > > + * the read to exclude a concurrent module load. > > > + */ > > > + if (btf_is_module(btf)) { > > > > So, this function is supplied with a module BTF, > > and the argument is that we can have an alive BTF reference > > but not to hold a live module reference? > > We have a convention (I don't know the history behind it) to always > btf_try_get_module() before reading associated kfunc sets, see a > comment in btf.c: > > /* Caution: > * Reference to the module (obtained using btf_try_get_module) > corresponding to > * the struct btf *MUST* be held when calling this function from verifier > * context. This is usually true as we stash references in prog's > kfunc_btf_tab; > * keeping the reference for the duration of the call provides the > necessary > * protection for looking up a well-formed btf->kfunc_set_tab. > */ > > I don't think it's worth the risk violating this convention within the > fix, even though you may be right about it being unnecessary in principle. Ok, upon closer examination, if btf comes from prog->attach_btf the module reference is not held at this point, when this function is called from bpf_check_attach_target(). > > > > References to kfunc's modules are stored in the > > bpf_prog_aux->kfunc_btf_tab, and the following code from core.c > > suggests that module references would be present for as long > > as the program using BTF from the module is present: > > > > static void bpf_prog_free_deferred(struct work_struct *work) > > { > > ... > > bpf_free_kfunc_btf_tab(aux->kfunc_btf_tab); > > ... > > } > > > > ? > > > > > + mod = btf_try_get_module(btf); > > > + if (!mod) > > > + return NULL; > > > + } > > > + kfunc_flags = btf_kfunc_accumulated_flags(btf, func_id); > > > + module_put(mod); > > > + > > > + if (kfunc_flags & KF_IMPLICIT_ARGS) { > > > + name = btf_name_by_offset(btf, func->name_off); > > > + return find_kfunc_impl_proto(log, btf, name); > > > + } > > > + > > > + return btf_type_by_id(btf, func->type); > > > +} > > > + > > > int bpf_check_attach_target(struct bpf_verifier_log *log, > > > const struct bpf_prog *prog, > > > const struct bpf_prog *tgt_prog, > > > @@ -19121,8 +19159,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, > > > if (prog_extension && > > > btf_check_type_match(log, prog, btf, t)) > > > return -EINVAL; > > > - t = btf_type_by_id(btf, t->type); > > > - if (!btf_type_is_func_proto(t)) > > > + t = btf_attach_func_proto(log, btf, btf_id); > > > + if (!t || !btf_type_is_func_proto(t)) > > > > This part seems fine. > > > > > return -EINVAL; > > > > > > if ((prog->aux->saved_dst_prog_type || prog->aux->saved_dst_attach_type) && > > > @@ -19407,8 +19445,8 @@ int bpf_check_attach_btf_id_multi(struct btf *btf, struct bpf_prog *prog, u32 bt > > > return -EINVAL; > > > if (!btf_type_is_func(t)) > > > return -EINVAL; > > > - t = btf_type_by_id(btf, t->type); > > > - if (!btf_type_is_func_proto(t)) > > > + t = btf_attach_func_proto(NULL, btf, btf_id); > > > + if (!t || !btf_type_is_func_proto(t)) > > > return -EINVAL; > > > err = btf_distill_func_proto(NULL, btf, t, tname, &tgt_info->fmodel); > > > if (err < 0) > > > -- > > > 2.55.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf v2 2/2] selftests/bpf: Cover tracing implicit kfunc args 2026-07-10 19:29 [PATCH bpf v2 0/2] bpf: Fix tracing of kfuncs with implicit args Ihor Solodrai 2026-07-10 19:29 ` [PATCH bpf v2 1/2] " Ihor Solodrai @ 2026-07-10 19:29 ` Ihor Solodrai 1 sibling, 0 replies; 6+ messages in thread From: Ihor Solodrai @ 2026-07-10 19:29 UTC (permalink / raw) To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Eduard Zingerman, Kumar Kartikeya Dwivedi Cc: Tejun Heo, bpf, linux-kernel, kernel-team From: Kumar Kartikeya Dwivedi <memxor@gmail.com> KF_IMPLICIT_ARGS kfuncs have a BPF-call prototype and a real kernel target prototype. Add a tracing selftest that attaches fentry and fexit programs to bpf_kfunc_implicit_arg(), runs a syscall BPF program that calls it, and checks that the tracing context exposes both the explicit argument and the implicit prog aux pointer. Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> Co-developed-by: Ihor Solodrai <ihor.solodrai@linux.dev> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- .../prog_tests/kfunc_implicit_args_tracing.c | 36 +++++++++ .../bpf/progs/kfunc_implicit_args_tracing.c | 77 +++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/kfunc_implicit_args_tracing.c create mode 100644 tools/testing/selftests/bpf/progs/kfunc_implicit_args_tracing.c diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_implicit_args_tracing.c b/tools/testing/selftests/bpf/prog_tests/kfunc_implicit_args_tracing.c new file mode 100644 index 000000000000..61cc5aaba025 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_implicit_args_tracing.c @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */ + +#include <test_progs.h> +#include "kfunc_implicit_args_tracing.skel.h" + +void test_kfunc_implicit_args_tracing(void) +{ + struct kfunc_implicit_args_tracing *skel; + LIBBPF_OPTS(bpf_test_run_opts, topts); + int err, fd; + + skel = kfunc_implicit_args_tracing__open_and_load(); + if (!ASSERT_OK_PTR(skel, "open_and_load")) + return; + + err = kfunc_implicit_args_tracing__attach(skel); + if (!ASSERT_OK(err, "attach")) + goto cleanup; + + fd = bpf_program__fd(skel->progs.trigger_implicit_arg); + err = bpf_prog_test_run_opts(fd, &topts); + if (!ASSERT_OK(err, "test_run")) + goto cleanup; + + ASSERT_EQ(topts.retval, 5, "kfunc_retval"); + ASSERT_EQ(skel->bss->fentry_arg_cnt, 2, "fentry_arg_cnt"); + ASSERT_NEQ(skel->bss->fentry_aux_arg, 0, "fentry_aux_arg"); + ASSERT_EQ(skel->bss->fentry_result, 1, "fentry_result"); + ASSERT_EQ(skel->bss->fexit_arg_cnt, 2, "fexit_arg_cnt"); + ASSERT_NEQ(skel->bss->fexit_aux_arg, 0, "fexit_aux_arg"); + ASSERT_EQ(skel->bss->fexit_result, 1, "fexit_result"); + +cleanup: + kfunc_implicit_args_tracing__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/kfunc_implicit_args_tracing.c b/tools/testing/selftests/bpf/progs/kfunc_implicit_args_tracing.c new file mode 100644 index 000000000000..995f8b8b5b9e --- /dev/null +++ b/tools/testing/selftests/bpf/progs/kfunc_implicit_args_tracing.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */ + +#include <vmlinux.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <errno.h> + +extern int bpf_kfunc_implicit_arg(int a) __weak __ksym; + +char _license[] SEC("license") = "GPL"; + +/* Shared arg checks; reports arg count and aux, returns 1 on success. */ +static __always_inline __u64 +check_implicit_args(void *ctx, __u64 *arg_cnt, __u64 *aux_arg) +{ + __u64 a = 0, aux = 0, z = 0; + __u64 result; + __s64 err; + + *arg_cnt = bpf_get_func_arg_cnt(ctx); + result = *arg_cnt == 2; + + err = bpf_get_func_arg(ctx, 0, &a); + result &= err == 0 && (int)a == 5; + + err = bpf_get_func_arg(ctx, 1, &aux); + *aux_arg = aux; + result &= err == 0 && aux != 0; + + err = bpf_get_func_arg(ctx, 2, &z); + result &= err == -EINVAL; + + return result; +} + +__u64 fentry_result; +__u64 fentry_arg_cnt; +__u64 fentry_aux_arg; + +SEC("fentry/bpf_kfunc_implicit_arg") +int BPF_PROG(trace_implicit_arg_fentry) +{ + __u64 ret = 0; + __s64 err; + + fentry_result = check_implicit_args(ctx, &fentry_arg_cnt, &fentry_aux_arg); + + err = bpf_get_func_ret(ctx, &ret); + fentry_result &= err == -EOPNOTSUPP; + + return 0; +} + +__u64 fexit_result; +__u64 fexit_arg_cnt; +__u64 fexit_aux_arg; + +SEC("fexit/bpf_kfunc_implicit_arg") +int BPF_PROG(trace_implicit_arg_fexit) +{ + __u64 ret = 0; + __s64 err; + + fexit_result = check_implicit_args(ctx, &fexit_arg_cnt, &fexit_aux_arg); + + err = bpf_get_func_ret(ctx, &ret); + fexit_result &= err == 0 && ret == 5; + + return 0; +} + +SEC("syscall") +int trigger_implicit_arg(void *ctx) +{ + return bpf_kfunc_implicit_arg(5); +} -- 2.55.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-11 23:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-10 19:29 [PATCH bpf v2 0/2] bpf: Fix tracing of kfuncs with implicit args Ihor Solodrai 2026-07-10 19:29 ` [PATCH bpf v2 1/2] " Ihor Solodrai 2026-07-11 10:43 ` Eduard Zingerman 2026-07-11 17:57 ` Ihor Solodrai 2026-07-11 23:51 ` Eduard Zingerman 2026-07-10 19:29 ` [PATCH bpf v2 2/2] selftests/bpf: Cover tracing implicit kfunc args Ihor Solodrai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox