[PATCH bpf v2 0/2] bpf: Fix tracing of kfuncs with implicit args

[PATCH bpf v2 0/2] bpf: Fix tracing of kfuncs with implicit args

[Ihor Solodrai]

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

[PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args

[Ihor Solodrai]

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

Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args

[Eduard Zingerman]

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

Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args

[Ihor Solodrai]

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

Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args

[Eduard Zingerman]

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

Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args

[Ihor Solodrai]

On 2026-07-11 4:51 p.m., Eduard Zingerman wrote:
> 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:
>>>
>>> [...]
>>>
>>> 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?

That would mean a bug in a kfunc set declaration, yeah.

> 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.

It was silly of me to mention the analysis as an obstacle (which I'm
not excited to do, sure). We are at -rc3, so there is a couple of
weeks still if we are targeting 7.2

But resolve_btfids series is certainly a merge-window thing, so the
fix shouldn't depend on it. And even carving out build-time KF_ flag
consistentcy enforcement from there is too potentially disruptive for
a "fix" IMO.

How about the following plan.

In the fix:
   * delete btf_kfunc_accumulated_flags()
   * expose (back) btf_kfunc_id_set_contains() from btf.c
   * in btf_attach_func_proto() walk the hooks and specifically check
     for *consistent* KF_IMPLICIT_ARGS
   * -EINVAL on inconsistency

In the v2 of resolve_btfids series (targeting 7.3):
   * implement kbuild/module-time enforcement of consistency
     for *all* KF_ flags
   * fix in-tree cases where that's not true
   * if there is a valid use-case, it should show up as a blocker, but
     I'll also eyeball all the kfunc set declarations

Does this make sense?

> 
>> 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.
>>
>>>
>>>> [...]

Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args

[Eduard Zingerman]

On Mon, 2026-07-13 at 09:48 -0700, Ihor Solodrai wrote:

[...]

> It was silly of me to mention the analysis as an obstacle (which I'm
> not excited to do, sure). We are at -rc3, so there is a couple of
> weeks still if we are targeting 7.2
> 
> But resolve_btfids series is certainly a merge-window thing, so the
> fix shouldn't depend on it. And even carving out build-time KF_ flag
> consistentcy enforcement from there is too potentially disruptive for
> a "fix" IMO.
> 
> How about the following plan.
> 
> In the fix:
>    * delete btf_kfunc_accumulated_flags()
>    * expose (back) btf_kfunc_id_set_contains() from btf.c
>    * in btf_attach_func_proto() walk the hooks and specifically check
>      for *consistent* KF_IMPLICIT_ARGS
>    * -EINVAL on inconsistency
> 
> In the v2 of resolve_btfids series (targeting 7.3):
>    * implement kbuild/module-time enforcement of consistency
>      for *all* KF_ flags
>    * fix in-tree cases where that's not true
>    * if there is a valid use-case, it should show up as a blocker, but
>      I'll also eyeball all the kfunc set declarations
> 
> Does this make sense?

You can do it like that, but tbh I that think doing it in two phases
complicates the process unnecessarily.

Re: [PATCH bpf v2 1/2] bpf: Fix tracing of kfuncs with implicit args

[Jiri Olsa]

On Fri, Jul 10, 2026 at 12:29:39PM -0700, Ihor Solodrai wrote:

SNIP

> +
>  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;

the btf_type_is_func check is now part of btf_attach_func_proto,
so I think we can remove this one

jirka

> -	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
> 
> 

[PATCH bpf v2 2/2] selftests/bpf: Cover tracing implicit kfunc args

[Ihor Solodrai]

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