public inbox for dwarves@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v1 0/8] bpf: magic kernel functions
@ 2025-10-29 19:01 Ihor Solodrai
  2025-10-29 19:01 ` [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros Ihor Solodrai
                   ` (8 more replies)
  0 siblings, 9 replies; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

This series develops the idea of implicit prog_aux argument for kfuncs
[1] into a generic "magic kfuncs" feature.

A mechanism is created for kfuncs to have arguments that are not
visible to the BPF programs, and are provided to the kernel function
implementation by the verifier.

This mechanism is then used in kfuncs that have an argument with
__prog annotation [2], which is the current way of passing struct
bpf_prog_aux pointer to kfuncs.

==== "Magic" ???

The usage of term "magic" is up for debate of course, I am open to
suggestions. I used it as a placeholder first and now it weirdly makes
sense. After all, "bpf" by itself doesn't mean anything either.

The feature effectively produces two variants of a kfunc
signature/prototype: one with the full argument list, and another with
particular arguments omitted.

There are many terms that in other contexts are used to describe
similar properties: implicit, optional, virtual, overloaded,
polymorphic, interface etc.  None of them is quite right for this use
case in BPF, and may trigger incorrect intuition if used.

An accurate term could be something like "verifier provided arguments"
and "kfuncs with verifier provided arguments", but that's too long for
usage in the identifiers.  "Magic" on the other hand is a short and
ambiguous adjective, which hopefully will prompt people to check the
documentation.

==== Implementation

pahole's BTF encoding is changed [3] to detect magic kfuncs and emit
two BTF functions from a single kfunc declaration:
  * kfunc_impl() with the arguments matching kernel declaration
  * kfunc() with __magic arguments omitted

BPF programs then can use both variants of the kfunc (to preserve
backwards compatibility for BPF programs that include vmlinux.h),
although non-_impl variant would be a preferred API.

To achieve this a few pieces of information must be possible to
decode in pahole from kernel side kfunc declaration:
  * which kfuncs are magic
  * what arguments should be omitted

A simple way to mark magic kfuncs is with a KF_MAGIC_ARGS flag.

As for the arguments, I considered a couple of options:
  * look at argument types and hardcode a list of types in pahole
  * have multiple KF_MAGIC_ARGS_<N> flags, which would indicate the
    number of arguments omitted
  * use a special arg name suffix (annotation), i.e. "__magic"

Of the three, the last one seems to be the simplest and most flexible.

It is also necessary for pahole to use conventional names for the
emitted kfunc pair for the verifier to be able to recognize them.

These changes in BTF create discrepancies in the verifier:
  * kfunc() now has an incorrect BTF function prototype
  * kfunc_impl() doesn't have a corresponding ksym and BTF flags

In order to handle them correctly, it's necessary to be able to lookup
kfunc() <-> kfunc_impl() pairs efficiently. Naive string lookup in BTF
is possible, but it is slow, which may negatively impact verification
performance for programs using relevant kfuncs.

Since the magic kfuncs are constant within a kernel, and their names
in BTF are conventional, we can define a constant table of magic
kfuncs using existing BTF_ID_LIST mechanism. A `magic_kfuncs` BTF ids
table is therefore defined and used for efficient lookups.

An inconvenience of the implementation described above is that the
writers of a magic kfunc have to do a couple of things:
  * mark the kfunc with KF_MAGIC_ARGS
  * mark the args with __magic annotation
  * add kfunc to magic_kfuncs table

Another one is that for special kfuncs the relevant checks in the
verifier must test for both original and _impl funcs. See changes to
bpf_wq_set_callback_impl for an example.

==== Testing

A number of selftests are already using aux__prog -> magic kfuncs.

Successful BPF CI run with modified pahole:
https://github.com/kernel-patches/bpf/actions/runs/18918350607

[1] https://lore.kernel.org/bpf/20250924211716.1287715-1-ihor.solodrai@linux.dev/
[2] https://docs.kernel.org/bpf/kfuncs.html#prog-annotation
[3] https://github.com/theihor/dwarves/tree/magic-args.draft

Ihor Solodrai (8):
  bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros
  bpf: Refactor btf_kfunc_id_set_contains
  bpf: Support for kfuncs with KF_MAGIC_ARGS
  bpf: Support __magic prog_aux arguments for kfuncs
  bpf: Re-define bpf_wq_set_callback as magic kfunc
  bpf,docs: Document KF_MAGIC_ARGS flag and __magic annotation
  bpf: Re-define bpf_task_work_schedule_* kfuncs as magic
  bpf: Re-define bpf_stream_vprintk as a magic kfunc

 Documentation/bpf/kfuncs.rst                  |  49 ++++-
 include/linux/btf.h                           |   7 +-
 include/linux/btf_ids.h                       |  10 ++
 kernel/bpf/btf.c                              |  70 ++++++--
 kernel/bpf/helpers.c                          |  31 ++--
 kernel/bpf/stream.c                           |   9 +-
 kernel/bpf/verifier.c                         | 167 ++++++++++++++++--
 tools/lib/bpf/bpf_helpers.h                   |   7 +-
 .../testing/selftests/bpf/bpf_experimental.h  |   5 -
 .../testing/selftests/bpf/progs/file_reader.c |   2 +-
 .../testing/selftests/bpf/progs/stream_fail.c |   6 +-
 tools/testing/selftests/bpf/progs/task_work.c |   6 +-
 .../selftests/bpf/progs/task_work_fail.c      |   8 +-
 .../selftests/bpf/progs/task_work_stress.c    |   2 +-
 .../bpf/progs/verifier_async_cb_context.c     |   4 +-
 tools/testing/selftests/bpf/progs/wq.c        |   2 +-
 .../testing/selftests/bpf/progs/wq_failures.c |   4 +-
 17 files changed, 312 insertions(+), 77 deletions(-)

-- 
2.51.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
@ 2025-10-29 19:01 ` Ihor Solodrai
  2025-10-29 19:41   ` bot+bpf-ci
  2025-10-29 23:54   ` Eduard Zingerman
  2025-10-29 19:01 ` [PATCH bpf-next v1 2/8] bpf: Refactor btf_kfunc_id_set_contains Ihor Solodrai
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

Implement macros in btf_ids.h to enable a calculation of BTF_ID_LIST
size. This is done by declaring an additional __end symbol which can
then be used as an indicator of the end of an array.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 include/linux/btf_ids.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index 139bdececdcf..27a4724d5aa9 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -97,6 +97,16 @@ asm(							\
 __BTF_ID_LIST(name, local)				\
 extern u32 name[];
 
+/*
+ * The BTF_ID_LIST_END macro may be used to denote an end
+ * of a BTF_ID_LIST. This enables calculation of the list
+ * size with BTF_ID_LIST_SIZE.
+ */
+#define BTF_ID_LIST_END(name) \
+BTF_ID_LIST(name##__end)
+#define BTF_ID_LIST_SIZE(name) \
+(name##__end - name)
+
 #define BTF_ID_LIST_GLOBAL(name, n)			\
 __BTF_ID_LIST(name, globl)
 
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH bpf-next v1 2/8] bpf: Refactor btf_kfunc_id_set_contains
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
  2025-10-29 19:01 ` [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros Ihor Solodrai
@ 2025-10-29 19:01 ` Ihor Solodrai
  2025-10-29 23:55   ` Eduard Zingerman
  2025-10-29 19:01 ` [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS Ihor Solodrai
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

btf_kfunc_id_set_contains() is called by fetch_kfunc_meta() in the BPF
verifier to get the kfunc flags stored in the .BTF_ids ELF section.
If it returns NULL instead of a valid pointer, it's interpreted by the
verifier as an illegal kfunc usage which fails the verification.

Conceptually, there are two potential reasons for
btf_kfunc_id_set_contains() to return NULL:

  1. Provided kfunc BTF id is not present in relevant kfunc id sets.
  2. The kfunc is not allowed, as determined by the program type
     specific filter [1].

The filter functions accept a pointer to `struct bpf_prog`, so they
might implicitly depend on earlier stages of verification, when
bpf_prog members are set.

For example, bpf_qdisc_kfunc_filter() in linux/net/sched/bpf_qdisc.c
inspects prog->aux->st_ops [2], which is initialized in:

    check_attach_btf_id() -> check_struct_ops_btf_id()

So far this hasn't been an issue, because fetch_kfunc_meta() is the
only place where lookup + filter logic is applied to a kfunc id.

However in subsequent patches of this series it is necessary to
inspect kfunc flags earlier in BPF verifier, in the add_kfunc_call().

To resolve this, refactor btf_kfunc_id_set_contains() into two
interface functions: btf_kfunc_flags() that does not apply the
filters, and btf_kfunc_flags_if_allowed() that does.

[1] https://lore.kernel.org/all/20230519225157.760788-7-aditi.ghag@isovalent.com/
[2] https://lore.kernel.org/all/20250409214606.2000194-4-ameryhung@gmail.com/

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 include/linux/btf.h   |  6 ++--
 kernel/bpf/btf.c      | 70 ++++++++++++++++++++++++++++++++-----------
 kernel/bpf/verifier.c |  2 +-
 3 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index f06976ffb63f..9c64bc5e5789 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -575,8 +575,10 @@ const char *btf_name_by_offset(const struct btf *btf, u32 offset);
 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_id_set_contains(const struct btf *btf, u32 kfunc_btf_id,
-			       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_flags_if_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);
 int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 0de8fc8a0e0b..4d31b8061daf 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8640,24 +8640,17 @@ static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
 	return ret;
 }
 
-static u32 *__btf_kfunc_id_set_contains(const struct btf *btf,
-					enum btf_kfunc_hook hook,
-					u32 kfunc_btf_id,
-					const struct bpf_prog *prog)
+static u32 *btf_kfunc_id_set_contains(const struct btf *btf,
+				      enum btf_kfunc_hook hook,
+				      u32 kfunc_btf_id)
 {
-	struct btf_kfunc_hook_filter *hook_filter;
 	struct btf_id_set8 *set;
-	u32 *id, i;
+	u32 *id;
 
 	if (hook >= BTF_KFUNC_HOOK_MAX)
 		return NULL;
 	if (!btf->kfunc_set_tab)
 		return NULL;
-	hook_filter = &btf->kfunc_set_tab->hook_filters[hook];
-	for (i = 0; i < hook_filter->nr_filters; i++) {
-		if (hook_filter->filters[i](prog, kfunc_btf_id))
-			return NULL;
-	}
 	set = btf->kfunc_set_tab->sets[hook];
 	if (!set)
 		return NULL;
@@ -8668,6 +8661,28 @@ static u32 *__btf_kfunc_id_set_contains(const struct btf *btf,
 	return id + 1;
 }
 
+static bool btf_kfunc_is_allowed(const struct btf *btf,
+				 enum btf_kfunc_hook hook,
+				 u32 kfunc_btf_id,
+				 const struct bpf_prog *prog)
+{
+	struct btf_kfunc_hook_filter *hook_filter;
+	int i;
+
+	if (hook >= BTF_KFUNC_HOOK_MAX)
+		return false;
+	if (!btf->kfunc_set_tab)
+		return false;
+
+	hook_filter = &btf->kfunc_set_tab->hook_filters[hook];
+	for (i = 0; i < hook_filter->nr_filters; i++) {
+		if (hook_filter->filters[i](prog, kfunc_btf_id))
+			return false;
+	}
+
+	return true;
+}
+
 static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
 {
 	switch (prog_type) {
@@ -8721,26 +8736,47 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
  * keeping the reference for the duration of the call provides the necessary
  * protection for looking up a well-formed btf->kfunc_set_tab.
  */
-u32 *btf_kfunc_id_set_contains(const struct btf *btf,
-			       u32 kfunc_btf_id,
-			       const struct bpf_prog *prog)
+u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_prog *prog)
 {
 	enum bpf_prog_type prog_type = resolve_prog_type(prog);
 	enum btf_kfunc_hook hook;
 	u32 *kfunc_flags;
 
-	kfunc_flags = __btf_kfunc_id_set_contains(btf, BTF_KFUNC_HOOK_COMMON, kfunc_btf_id, prog);
+	kfunc_flags = btf_kfunc_id_set_contains(btf, BTF_KFUNC_HOOK_COMMON, kfunc_btf_id);
 	if (kfunc_flags)
 		return kfunc_flags;
 
 	hook = bpf_prog_type_to_kfunc_hook(prog_type);
-	return __btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id, prog);
+	return btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id);
+}
+
+u32 *btf_kfunc_flags_if_allowed(const struct btf *btf,
+				u32 kfunc_btf_id,
+				const struct bpf_prog *prog)
+{
+	enum bpf_prog_type prog_type = resolve_prog_type(prog);
+	enum btf_kfunc_hook hook;
+	u32 *kfunc_flags;
+
+	kfunc_flags = btf_kfunc_id_set_contains(btf, BTF_KFUNC_HOOK_COMMON, kfunc_btf_id);
+	if (kfunc_flags && btf_kfunc_is_allowed(btf, BTF_KFUNC_HOOK_COMMON, kfunc_btf_id, prog))
+		return kfunc_flags;
+
+	hook = bpf_prog_type_to_kfunc_hook(prog_type);
+	kfunc_flags = btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id);
+	if (kfunc_flags && btf_kfunc_is_allowed(btf, hook, kfunc_btf_id, prog))
+		return kfunc_flags;
+
+	return NULL;
 }
 
 u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id,
 				const struct bpf_prog *prog)
 {
-	return __btf_kfunc_id_set_contains(btf, BTF_KFUNC_HOOK_FMODRET, kfunc_btf_id, prog);
+	if (!btf_kfunc_is_allowed(btf, BTF_KFUNC_HOOK_FMODRET, kfunc_btf_id, prog))
+		return NULL;
+
+	return btf_kfunc_id_set_contains(btf, BTF_KFUNC_HOOK_FMODRET, kfunc_btf_id);
 }
 
 static int __register_btf_kfunc_id_set(enum btf_kfunc_hook hook,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 542e23fb19c7..cb1b483be0fa 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13631,7 +13631,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
 		*kfunc_name = func_name;
 	func_proto = btf_type_by_id(desc_btf, func->type);
 
-	kfunc_flags = btf_kfunc_id_set_contains(desc_btf, func_id, env->prog);
+	kfunc_flags = btf_kfunc_flags_if_allowed(desc_btf, func_id, env->prog);
 	if (!kfunc_flags) {
 		return -EACCES;
 	}
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
  2025-10-29 19:01 ` [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros Ihor Solodrai
  2025-10-29 19:01 ` [PATCH bpf-next v1 2/8] bpf: Refactor btf_kfunc_id_set_contains Ihor Solodrai
@ 2025-10-29 19:01 ` Ihor Solodrai
  2025-10-29 19:41   ` bot+bpf-ci
                     ` (4 more replies)
  2025-10-29 19:01 ` [PATCH bpf-next v1 4/8] bpf: Support __magic prog_aux arguments for kfuncs Ihor Solodrai
                   ` (5 subsequent siblings)
  8 siblings, 5 replies; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

A kernel function bpf_foo with KF_MAGIC_ARGS flag is expected to have
two types in BTF:
  * `bpf_foo` with a function prototype that omits __magic arguments
  * `bpf_foo_impl` with a function prototype that matches kernel
     declaration, but doesn't have a ksym associated with its name

In order to support magic kfuncs the verifier has to know how to
resolve calls both of `bpf_foo` and `bpf_foo_impl` to the correct BTF
function prototype and address.

In add_kfunc_call() kfunc flags are inspected to detect a magic kfunc
or its _impl, and then the address and func_proto are adjusted for the
kfunc descriptor.

In fetch_kfunc_meta() similar logic is used to fixup the contents of
struct bpf_kfunc_call_arg_meta.

In check_kfunc_call() reset the subreg_def of registers holding magic
arguments to correctly track zero extensions.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 include/linux/btf.h   |   1 +
 kernel/bpf/verifier.c | 123 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index 9c64bc5e5789..3fe20514692f 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -79,6 +79,7 @@
 #define KF_ARENA_RET    (1 << 13) /* kfunc returns an arena pointer */
 #define KF_ARENA_ARG1   (1 << 14) /* kfunc takes an arena pointer as its first argument */
 #define KF_ARENA_ARG2   (1 << 15) /* kfunc takes an arena pointer as its second argument */
+#define KF_MAGIC_ARGS   (1 << 16) /* kfunc signature is different from its BPF signature */
 
 /*
  * Tag marking a kernel function as a kfunc. This is meant to minimize the
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index cb1b483be0fa..fcf0872b9e3d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3263,17 +3263,68 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
 	return btf_vmlinux ?: ERR_PTR(-ENOENT);
 }
 
+/*
+ * magic_kfuncs is used as a list of (foo, foo_impl) pairs
+ */
+BTF_ID_LIST(magic_kfuncs)
+BTF_ID_UNUSED
+BTF_ID_LIST_END(magic_kfuncs)
+
+static s32 magic_kfunc_by_impl(s32 impl_func_id)
+{
+	int i;
+
+	for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
+		if (magic_kfuncs[i] == impl_func_id)
+			return magic_kfuncs[i - 1];
+	}
+	return -ENOENT;
+}
+
+static s32 impl_by_magic_kfunc(s32 func_id)
+{
+	int i;
+
+	for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
+		if (magic_kfuncs[i] == func_id)
+			return magic_kfuncs[i + 1];
+	}
+	return -ENOENT;
+}
+
+static const struct btf_type *find_magic_kfunc_proto(struct btf *desc_btf, s32 func_id)
+{
+	const struct btf_type *impl_func, *func_proto;
+	u32 impl_func_id;
+
+	impl_func_id = impl_by_magic_kfunc(func_id);
+	if (impl_func_id < 0)
+		return NULL;
+
+	impl_func = btf_type_by_id(desc_btf, impl_func_id);
+	if (!impl_func || !btf_type_is_func(impl_func))
+		return NULL;
+
+	func_proto = btf_type_by_id(desc_btf, impl_func->type);
+	if (!func_proto || !btf_type_is_func_proto(func_proto))
+		return NULL;
+
+	return func_proto;
+}
+
 static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
 {
-	const struct btf_type *func, *func_proto;
+	const struct btf_type *func, *func_proto, *tmp_func;
 	struct bpf_kfunc_btf_tab *btf_tab;
+	const char *func_name, *tmp_name;
 	struct btf_func_model func_model;
 	struct bpf_kfunc_desc_tab *tab;
 	struct bpf_prog_aux *prog_aux;
 	struct bpf_kfunc_desc *desc;
-	const char *func_name;
 	struct btf *desc_btf;
 	unsigned long addr;
+	u32 *kfunc_flags;
+	s32 tmp_func_id;
 	int err;
 
 	prog_aux = env->prog->aux;
@@ -3349,8 +3400,37 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
 		return -EINVAL;
 	}
 
+	kfunc_flags = btf_kfunc_flags(desc_btf, func_id, env->prog);
 	func_name = btf_name_by_offset(desc_btf, func->name_off);
 	addr = kallsyms_lookup_name(func_name);
+
+	/* This may be an _impl kfunc with KF_MAGIC_ARGS counterpart */
+	if (unlikely(!addr && !kfunc_flags)) {
+		tmp_func_id = magic_kfunc_by_impl(func_id);
+		if (tmp_func_id < 0)
+			return -EACCES;
+		tmp_func = btf_type_by_id(desc_btf, tmp_func_id);
+		if (!tmp_func || !btf_type_is_func(tmp_func))
+			return -EACCES;
+		tmp_name = btf_name_by_offset(desc_btf, tmp_func->name_off);
+		addr = kallsyms_lookup_name(tmp_name);
+	}
+
+	/*
+	 * Note that kfunc_flags may be NULL at this point, which means that we couldn't find
+	 * func_id in any relevant kfunc_id_set. This most likely indicates an invalid kfunc call.
+	 * However we don't want to fail the verification here, because invalid calls may be
+	 * eliminated as dead code later.
+	 */
+	if (unlikely(kfunc_flags && KF_MAGIC_ARGS & *kfunc_flags)) {
+		func_proto = find_magic_kfunc_proto(desc_btf, func_id);
+		if (!func_proto) {
+			verbose(env, "cannot find _impl proto for kernel function %s\n",
+			func_name);
+			return -EINVAL;
+		}
+	}
+
 	if (!addr) {
 		verbose(env, "cannot find address for kernel function %s\n",
 			func_name);
@@ -12051,6 +12131,11 @@ static bool is_kfunc_arg_irq_flag(const struct btf *btf, const struct btf_param
 	return btf_param_match_suffix(btf, arg, "__irq_flag");
 }
 
+static bool is_kfunc_arg_magic(const struct btf *btf, const struct btf_param *arg)
+{
+	return btf_param_match_suffix(btf, arg, "__magic");
+}
+
 static bool is_kfunc_arg_prog(const struct btf *btf, const struct btf_param *arg)
 {
 	return btf_param_match_suffix(btf, arg, "__prog");
@@ -13613,6 +13698,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
 	u32 func_id, *kfunc_flags;
 	const char *func_name;
 	struct btf *desc_btf;
+	s32 tmp_func_id;
 
 	if (kfunc_name)
 		*kfunc_name = NULL;
@@ -13632,10 +13718,28 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
 	func_proto = btf_type_by_id(desc_btf, func->type);
 
 	kfunc_flags = btf_kfunc_flags_if_allowed(desc_btf, func_id, env->prog);
-	if (!kfunc_flags) {
-		return -EACCES;
+	if (unlikely(!kfunc_flags)) {
+		/*
+		 * An _impl kfunc with KF_MAGIC_ARGS counterpart
+		 * does not have its own kfunc flags.
+		 */
+		tmp_func_id = magic_kfunc_by_impl(func_id);
+		if (tmp_func_id < 0)
+			return -EACCES;
+		kfunc_flags = btf_kfunc_flags_if_allowed(desc_btf, tmp_func_id, env->prog);
+		if (!kfunc_flags)
+			return -EACCES;
+	} else if (unlikely(KF_MAGIC_ARGS & *kfunc_flags)) {
+		/*
+		 * An actual func_proto of a kfunc with KF_MAGIC_ARGS flag
+		 * can be found through the corresponding _impl kfunc.
+		 */
+		func_proto = find_magic_kfunc_proto(desc_btf, func_id);
 	}
 
+	if (!func_proto)
+		return -EACCES;
+
 	memset(meta, 0, sizeof(*meta));
 	meta->btf = desc_btf;
 	meta->func_id = func_id;
@@ -14189,6 +14293,17 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	for (i = 0; i < nargs; i++) {
 		u32 regno = i + 1;
 
+		/*
+		 * Magic arguments are set after main verification pass.
+		 * For correct tracking of zero-extensions we have to reset subreg_def for such
+		 * args. Otherwise mark_btf_func_reg_size() will be inspecting subreg_def of regs
+		 * from an earlier (irrelevant) point in the program, which may lead to an error
+		 * in opt_subreg_zext_lo32_rnd_hi32().
+		 */
+		if (unlikely(KF_MAGIC_ARGS & meta.kfunc_flags
+				&& is_kfunc_arg_magic(desc_btf, &args[i])))
+			regs[regno].subreg_def = DEF_NOT_SUBREG;
+
 		t = btf_type_skip_modifiers(desc_btf, args[i].type, NULL);
 		if (btf_type_is_ptr(t))
 			mark_btf_func_reg_size(env, regno, sizeof(void *));
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH bpf-next v1 4/8] bpf: Support __magic prog_aux arguments for kfuncs
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
                   ` (2 preceding siblings ...)
  2025-10-29 19:01 ` [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS Ihor Solodrai
@ 2025-10-29 19:01 ` Ihor Solodrai
  2025-10-29 19:01 ` [PATCH bpf-next v1 5/8] bpf: Re-define bpf_wq_set_callback as magic kfunc Ihor Solodrai
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

Teach the verifier that the prog_aux argument of a kfunc can be
specified with __magic suffix, in which case the type of the function
parameter must be checked.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 kernel/bpf/verifier.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fcf0872b9e3d..67914464d503 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12136,9 +12136,12 @@ static bool is_kfunc_arg_magic(const struct btf *btf, const struct btf_param *ar
 	return btf_param_match_suffix(btf, arg, "__magic");
 }
 
+static bool is_kfunc_arg_prog_aux(const struct btf *btf, const struct btf_param *arg);
+
 static bool is_kfunc_arg_prog(const struct btf *btf, const struct btf_param *arg)
 {
-	return btf_param_match_suffix(btf, arg, "__prog");
+	return btf_param_match_suffix(btf, arg, "__prog") ||
+	       (is_kfunc_arg_magic(btf, arg) && is_kfunc_arg_prog_aux(btf, arg));
 }
 
 static bool is_kfunc_arg_scalar_with_name(const struct btf *btf,
@@ -12169,6 +12172,7 @@ enum {
 	KF_ARG_WORKQUEUE_ID,
 	KF_ARG_RES_SPIN_LOCK_ID,
 	KF_ARG_TASK_WORK_ID,
+	KF_ARG_PROG_AUX_ID
 };
 
 BTF_ID_LIST(kf_arg_btf_ids)
@@ -12180,6 +12184,7 @@ BTF_ID(struct, bpf_rb_node)
 BTF_ID(struct, bpf_wq)
 BTF_ID(struct, bpf_res_spin_lock)
 BTF_ID(struct, bpf_task_work)
+BTF_ID(struct, bpf_prog_aux)
 
 static bool __is_kfunc_ptr_arg_type(const struct btf *btf,
 				    const struct btf_param *arg, int type)
@@ -12260,6 +12265,11 @@ static bool is_kfunc_arg_callback(struct bpf_verifier_env *env, const struct btf
 	return true;
 }
 
+static bool is_kfunc_arg_prog_aux(const struct btf *btf, const struct btf_param *arg)
+{
+	return __is_kfunc_ptr_arg_type(btf, arg, KF_ARG_PROG_AUX_ID);
+}
+
 /* Returns true if struct is composed of scalars, 4 levels of nesting allowed */
 static bool __btf_type_is_scalar_struct(struct bpf_verifier_env *env,
 					const struct btf *btf,
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH bpf-next v1 5/8] bpf: Re-define bpf_wq_set_callback as magic kfunc
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
                   ` (3 preceding siblings ...)
  2025-10-29 19:01 ` [PATCH bpf-next v1 4/8] bpf: Support __magic prog_aux arguments for kfuncs Ihor Solodrai
@ 2025-10-29 19:01 ` Ihor Solodrai
  2025-10-30  0:16   ` Eduard Zingerman
  2025-10-29 19:01 ` [PATCH bpf-next v1 6/8] bpf,docs: Document KF_MAGIC_ARGS flag and __magic annotation Ihor Solodrai
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

* Rename bpf_wq_set_callback_impl to bpf_wq_set_callback
* void *aux__prog => struct bpf_prog_aux *aux__magic
* Set KF_MAGIC_ARGS kfunc flag
* Add bpf_wq_set_callback and _impl to magic_kfuncs BTF_ID_LIST
* Update special kfunc checks in the verifier to accept both _impl and
  non-_impl BTF ids

In the selftests, a bpf_wq_set_callback_impl() call is intentionally
introduced to verify that both signatures are handled correctly.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 kernel/bpf/helpers.c                           | 13 ++++++-------
 kernel/bpf/verifier.c                          | 18 +++++++++++-------
 tools/testing/selftests/bpf/bpf_experimental.h |  5 -----
 tools/testing/selftests/bpf/progs/wq.c         |  2 +-
 .../testing/selftests/bpf/progs/wq_failures.c  |  4 ++--
 5 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 930e132f440f..ee56f74f70c1 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3119,18 +3119,17 @@ __bpf_kfunc int bpf_wq_start(struct bpf_wq *wq, unsigned int flags)
 	return 0;
 }
 
-__bpf_kfunc int bpf_wq_set_callback_impl(struct bpf_wq *wq,
-					 int (callback_fn)(void *map, int *key, void *value),
-					 unsigned int flags,
-					 void *aux__prog)
+__bpf_kfunc int bpf_wq_set_callback(struct bpf_wq *wq,
+				    int (callback_fn)(void *map, int *key, void *value),
+				    unsigned int flags,
+				    struct bpf_prog_aux *aux__magic)
 {
-	struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__prog;
 	struct bpf_async_kern *async = (struct bpf_async_kern *)wq;
 
 	if (flags)
 		return -EINVAL;
 
-	return __bpf_async_set_callback(async, callback_fn, aux, flags, BPF_ASYNC_TYPE_WQ);
+	return __bpf_async_set_callback(async, callback_fn, aux__magic, flags, BPF_ASYNC_TYPE_WQ);
 }
 
 __bpf_kfunc void bpf_preempt_disable(void)
@@ -4483,7 +4482,7 @@ BTF_ID_FLAGS(func, bpf_dynptr_memset)
 BTF_ID_FLAGS(func, bpf_modify_return_test_tp)
 #endif
 BTF_ID_FLAGS(func, bpf_wq_init)
-BTF_ID_FLAGS(func, bpf_wq_set_callback_impl)
+BTF_ID_FLAGS(func, bpf_wq_set_callback, KF_MAGIC_ARGS)
 BTF_ID_FLAGS(func, bpf_wq_start)
 BTF_ID_FLAGS(func, bpf_preempt_disable)
 BTF_ID_FLAGS(func, bpf_preempt_enable)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 67914464d503..3c9e963d879b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -512,7 +512,7 @@ static bool is_async_callback_calling_kfunc(u32 btf_id);
 static bool is_callback_calling_kfunc(u32 btf_id);
 static bool is_bpf_throw_kfunc(struct bpf_insn *insn);
 
-static bool is_bpf_wq_set_callback_impl_kfunc(u32 btf_id);
+static bool is_bpf_wq_set_callback_kfunc(u32 btf_id);
 static bool is_task_work_add_kfunc(u32 func_id);
 
 static bool is_sync_callback_calling_function(enum bpf_func_id func_id)
@@ -554,7 +554,7 @@ static bool is_async_cb_sleepable(struct bpf_verifier_env *env, struct bpf_insn
 
 	/* bpf_wq and bpf_task_work callbacks are always sleepable. */
 	if (bpf_pseudo_kfunc_call(insn) && insn->off == 0 &&
-	    (is_bpf_wq_set_callback_impl_kfunc(insn->imm) || is_task_work_add_kfunc(insn->imm)))
+	    (is_bpf_wq_set_callback_kfunc(insn->imm) || is_task_work_add_kfunc(insn->imm)))
 		return true;
 
 	verifier_bug(env, "unhandled async callback in is_async_cb_sleepable");
@@ -3267,7 +3267,8 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
  * magic_kfuncs is used as a list of (foo, foo_impl) pairs
  */
 BTF_ID_LIST(magic_kfuncs)
-BTF_ID_UNUSED
+BTF_ID(func, bpf_wq_set_callback)
+BTF_ID(func, bpf_wq_set_callback_impl)
 BTF_ID_LIST_END(magic_kfuncs)
 
 static s32 magic_kfunc_by_impl(s32 impl_func_id)
@@ -12385,6 +12386,7 @@ enum special_kfunc_type {
 	KF___bpf_trap,
 	KF_bpf_task_work_schedule_signal,
 	KF_bpf_task_work_schedule_resume,
+	KF_bpf_wq_set_callback,
 };
 
 BTF_ID_LIST(special_kfunc_list)
@@ -12459,6 +12461,7 @@ BTF_ID(func, bpf_dynptr_file_discard)
 BTF_ID(func, __bpf_trap)
 BTF_ID(func, bpf_task_work_schedule_signal)
 BTF_ID(func, bpf_task_work_schedule_resume)
+BTF_ID(func, bpf_wq_set_callback)
 
 static bool is_task_work_add_kfunc(u32 func_id)
 {
@@ -12906,7 +12909,7 @@ static bool is_sync_callback_calling_kfunc(u32 btf_id)
 
 static bool is_async_callback_calling_kfunc(u32 btf_id)
 {
-	return btf_id == special_kfunc_list[KF_bpf_wq_set_callback_impl] ||
+	return is_bpf_wq_set_callback_kfunc(btf_id) ||
 	       is_task_work_add_kfunc(btf_id);
 }
 
@@ -12916,9 +12919,10 @@ static bool is_bpf_throw_kfunc(struct bpf_insn *insn)
 	       insn->imm == special_kfunc_list[KF_bpf_throw];
 }
 
-static bool is_bpf_wq_set_callback_impl_kfunc(u32 btf_id)
+static bool is_bpf_wq_set_callback_kfunc(u32 btf_id)
 {
-	return btf_id == special_kfunc_list[KF_bpf_wq_set_callback_impl];
+	return btf_id == special_kfunc_list[KF_bpf_wq_set_callback_impl] ||
+	       btf_id == special_kfunc_list[KF_bpf_wq_set_callback];
 }
 
 static bool is_callback_calling_kfunc(u32 btf_id)
@@ -14035,7 +14039,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		meta.r0_rdonly = false;
 	}
 
-	if (is_bpf_wq_set_callback_impl_kfunc(meta.func_id)) {
+	if (is_bpf_wq_set_callback_kfunc(meta.func_id)) {
 		err = push_callback_call(env, insn, insn_idx, meta.subprogno,
 					 set_timer_callback_state);
 		if (err) {
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 2cd9165c7348..68a49b1f77ae 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -580,11 +580,6 @@ extern void bpf_iter_css_destroy(struct bpf_iter_css *it) __weak __ksym;
 
 extern int bpf_wq_init(struct bpf_wq *wq, void *p__map, unsigned int flags) __weak __ksym;
 extern int bpf_wq_start(struct bpf_wq *wq, unsigned int flags) __weak __ksym;
-extern int bpf_wq_set_callback_impl(struct bpf_wq *wq,
-		int (callback_fn)(void *map, int *key, void *value),
-		unsigned int flags__k, void *aux__ign) __ksym;
-#define bpf_wq_set_callback(timer, cb, flags) \
-	bpf_wq_set_callback_impl(timer, cb, flags, NULL)
 
 struct bpf_iter_kmem_cache;
 extern int bpf_iter_kmem_cache_new(struct bpf_iter_kmem_cache *it) __weak __ksym;
diff --git a/tools/testing/selftests/bpf/progs/wq.c b/tools/testing/selftests/bpf/progs/wq.c
index 25be2cd9d42c..f265242b954d 100644
--- a/tools/testing/selftests/bpf/progs/wq.c
+++ b/tools/testing/selftests/bpf/progs/wq.c
@@ -107,7 +107,7 @@ static int test_hmap_elem_callback(void *map, int *key,
 	if (bpf_wq_init(wq, map, 0) != 0)
 		return -3;
 
-	if (bpf_wq_set_callback(wq, callback_fn, 0))
+	if (bpf_wq_set_callback_impl(wq, callback_fn, 0, NULL))
 		return -4;
 
 	if (bpf_wq_start(wq, 0))
diff --git a/tools/testing/selftests/bpf/progs/wq_failures.c b/tools/testing/selftests/bpf/progs/wq_failures.c
index d06f6d40594a..3767f5595bbc 100644
--- a/tools/testing/selftests/bpf/progs/wq_failures.c
+++ b/tools/testing/selftests/bpf/progs/wq_failures.c
@@ -97,7 +97,7 @@ __failure
 /* check that the first argument of bpf_wq_set_callback()
  * is a correct bpf_wq pointer.
  */
-__msg(": (85) call bpf_wq_set_callback_impl#") /* anchor message */
+__msg(": (85) call bpf_wq_set_callback#") /* anchor message */
 __msg("arg#0 doesn't point to a map value")
 long test_wrong_wq_pointer(void *ctx)
 {
@@ -123,7 +123,7 @@ __failure
 /* check that the first argument of bpf_wq_set_callback()
  * is a correct bpf_wq pointer.
  */
-__msg(": (85) call bpf_wq_set_callback_impl#") /* anchor message */
+__msg(": (85) call bpf_wq_set_callback#") /* anchor message */
 __msg("off 1 doesn't point to 'struct bpf_wq' that is at 0")
 long test_wrong_wq_pointer_offset(void *ctx)
 {
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH bpf-next v1 6/8] bpf,docs: Document KF_MAGIC_ARGS flag and __magic annotation
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
                   ` (4 preceding siblings ...)
  2025-10-29 19:01 ` [PATCH bpf-next v1 5/8] bpf: Re-define bpf_wq_set_callback as magic kfunc Ihor Solodrai
@ 2025-10-29 19:01 ` Ihor Solodrai
  2025-10-30  0:21   ` Eduard Zingerman
  2025-10-29 19:01 ` [PATCH bpf-next v1 7/8] bpf: Re-define bpf_task_work_schedule_* kfuncs as magic Ihor Solodrai
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

Add sections explaining KF_MAGIC_ARGS kfunc flag and __magic argument
annotation. Mark __prog annotation as deprecated.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 Documentation/bpf/kfuncs.rst | 49 +++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
index e38941370b90..1a261f84e157 100644
--- a/Documentation/bpf/kfuncs.rst
+++ b/Documentation/bpf/kfuncs.rst
@@ -160,7 +160,7 @@ Or::
                 ...
         }
 
-2.2.6 __prog Annotation
+2.2.6 __prog Annotation (deprecated, use __magic instead)
 ---------------------------
 This annotation is used to indicate that the argument needs to be fixed up to
 the bpf_prog_aux of the caller BPF program. Any value passed into this argument
@@ -177,6 +177,37 @@ An example is given below::
                 ...
          }
 
+.. _magic_annotation:
+
+2.2.7 __magic Annotation
+---------------------------
+This annotation is used in kfuncs with KF_MAGIC_ARGS flag to indicate the
+arguments that are omitted in the BPF signature of the kfunc. The actual
+values for __magic arguments are set by the verifier at load time, and
+depend on the argument type.
+
+Currently only ``struct bpf_prog_aux *`` type is supported.
+
+Example declaration:
+
+.. code-block:: c
+
+	__bpf_kfunc int bpf_wq_set_callback(struct bpf_wq *wq,
+					    int (callback_fn)(void *map, int *key, void *value),
+					    unsigned int flags,
+					    struct bpf_prog_aux *aux__magic)
+	{
+		...
+	}
+
+Example usage:
+
+.. code-block:: c
+
+	/* note the last argument is omitted */
+	if (bpf_wq_set_callback(wq, callback_fn, 0))
+		return -1;
+
 .. _BPF_kfunc_nodef:
 
 2.3 Using an existing kernel function
@@ -374,6 +405,22 @@ encouraged to make their use-cases known as early as possible, and participate
 in upstream discussions regarding whether to keep, change, deprecate, or remove
 those kfuncs if and when such discussions occur.
 
+2.4.10 KF_MAGIC_ARGS flag
+------------------------------------
+
+The KF_MAGIC_ARGS flag is used to indicate that the BPF signature of the kfunc
+is different from it's kernel signature, and the values for arguments annotated
+with __magic suffix are provided at load time by the verifier.
+
+A kfunc with KF_MAGIC_ARGS flag therefore has two types in BTF: one function
+matching the kernel declaration (with _impl suffix by convention), and another
+matching the intended BPF API.
+
+Verifier allows calls to both _impl and non-_impl versions of a magic kfunc.
+
+Note that only arguments of particular types can be __magic.
+See :ref:`magic_annotation`.
+
 2.5 Registering the kfuncs
 --------------------------
 
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH bpf-next v1 7/8] bpf: Re-define bpf_task_work_schedule_* kfuncs as magic
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
                   ` (5 preceding siblings ...)
  2025-10-29 19:01 ` [PATCH bpf-next v1 6/8] bpf,docs: Document KF_MAGIC_ARGS flag and __magic annotation Ihor Solodrai
@ 2025-10-29 19:01 ` Ihor Solodrai
  2025-10-29 19:01 ` [PATCH bpf-next v1 8/8] bpf: Re-define bpf_stream_vprintk as a magic kfunc Ihor Solodrai
  2025-10-30  0:44 ` [PATCH bpf-next v1 0/8] bpf: magic kernel functions Eduard Zingerman
  8 siblings, 0 replies; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

* void *aux__prog => struct bpf_prog_aux *aux__magic
* Set KF_MAGIC_ARGS flag
* Add relevant symbols to magic_kfuncs list
* Update selftests to use the new signature

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 kernel/bpf/helpers.c                             | 16 ++++++++--------
 kernel/bpf/verifier.c                            | 12 +++++++++++-
 tools/testing/selftests/bpf/progs/file_reader.c  |  2 +-
 tools/testing/selftests/bpf/progs/task_work.c    |  6 +++---
 .../testing/selftests/bpf/progs/task_work_fail.c |  8 ++++----
 .../selftests/bpf/progs/task_work_stress.c       |  2 +-
 .../bpf/progs/verifier_async_cb_context.c        |  4 ++--
 7 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index ee56f74f70c1..6a095796433a 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4278,15 +4278,15 @@ static int bpf_task_work_schedule(struct task_struct *task, struct bpf_task_work
  * @tw: Pointer to struct bpf_task_work in BPF map value for internal bookkeeping
  * @map__map: bpf_map that embeds struct bpf_task_work in the values
  * @callback: pointer to BPF subprogram to call
- * @aux__prog: user should pass NULL
+ * @aux__magic: pointer to bpf_prog_aux of the caller BPF program, set by the verifier
  *
  * Return: 0 if task work has been scheduled successfully, negative error code otherwise
  */
 __bpf_kfunc int bpf_task_work_schedule_signal(struct task_struct *task, struct bpf_task_work *tw,
 					      void *map__map, bpf_task_work_callback_t callback,
-					      void *aux__prog)
+					      struct bpf_prog_aux *aux__magic)
 {
-	return bpf_task_work_schedule(task, tw, map__map, callback, aux__prog, TWA_SIGNAL);
+	return bpf_task_work_schedule(task, tw, map__map, callback, aux__magic, TWA_SIGNAL);
 }
 
 /**
@@ -4295,15 +4295,15 @@ __bpf_kfunc int bpf_task_work_schedule_signal(struct task_struct *task, struct b
  * @tw: Pointer to struct bpf_task_work in BPF map value for internal bookkeeping
  * @map__map: bpf_map that embeds struct bpf_task_work in the values
  * @callback: pointer to BPF subprogram to call
- * @aux__prog: user should pass NULL
+ * @aux__magic: pointer to bpf_prog_aux of the caller BPF program, set by the verifier
  *
  * Return: 0 if task work has been scheduled successfully, negative error code otherwise
  */
 __bpf_kfunc int bpf_task_work_schedule_resume(struct task_struct *task, struct bpf_task_work *tw,
 					      void *map__map, bpf_task_work_callback_t callback,
-					      void *aux__prog)
+					      struct bpf_prog_aux *aux__magic)
 {
-	return bpf_task_work_schedule(task, tw, map__map, callback, aux__prog, TWA_RESUME);
+	return bpf_task_work_schedule(task, tw, map__map, callback, aux__magic, TWA_RESUME);
 }
 
 static int make_file_dynptr(struct file *file, u32 flags, bool may_sleep,
@@ -4529,8 +4529,8 @@ BTF_ID_FLAGS(func, bpf_strncasestr);
 BTF_ID_FLAGS(func, bpf_cgroup_read_xattr, KF_RCU)
 #endif
 BTF_ID_FLAGS(func, bpf_stream_vprintk, KF_TRUSTED_ARGS)
-BTF_ID_FLAGS(func, bpf_task_work_schedule_signal, KF_TRUSTED_ARGS)
-BTF_ID_FLAGS(func, bpf_task_work_schedule_resume, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_task_work_schedule_signal, KF_MAGIC_ARGS | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_task_work_schedule_resume, KF_MAGIC_ARGS | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_dynptr_from_file, KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_dynptr_file_discard)
 BTF_KFUNCS_END(common_btf_ids)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 3c9e963d879b..ad4af5ddb523 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3269,6 +3269,10 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
 BTF_ID_LIST(magic_kfuncs)
 BTF_ID(func, bpf_wq_set_callback)
 BTF_ID(func, bpf_wq_set_callback_impl)
+BTF_ID(func, bpf_task_work_schedule_signal)
+BTF_ID(func, bpf_task_work_schedule_signal_impl)
+BTF_ID(func, bpf_task_work_schedule_resume)
+BTF_ID(func, bpf_task_work_schedule_resume_impl)
 BTF_ID_LIST_END(magic_kfuncs)
 
 static s32 magic_kfunc_by_impl(s32 impl_func_id)
@@ -12387,6 +12391,8 @@ enum special_kfunc_type {
 	KF_bpf_task_work_schedule_signal,
 	KF_bpf_task_work_schedule_resume,
 	KF_bpf_wq_set_callback,
+	KF_bpf_task_work_schedule_signal_impl,
+	KF_bpf_task_work_schedule_resume_impl,
 };
 
 BTF_ID_LIST(special_kfunc_list)
@@ -12462,11 +12468,15 @@ BTF_ID(func, __bpf_trap)
 BTF_ID(func, bpf_task_work_schedule_signal)
 BTF_ID(func, bpf_task_work_schedule_resume)
 BTF_ID(func, bpf_wq_set_callback)
+BTF_ID(func, bpf_task_work_schedule_signal_impl)
+BTF_ID(func, bpf_task_work_schedule_resume_impl)
 
 static bool is_task_work_add_kfunc(u32 func_id)
 {
 	return func_id == special_kfunc_list[KF_bpf_task_work_schedule_signal] ||
-	       func_id == special_kfunc_list[KF_bpf_task_work_schedule_resume];
+	       func_id == special_kfunc_list[KF_bpf_task_work_schedule_signal_impl] ||
+	       func_id == special_kfunc_list[KF_bpf_task_work_schedule_resume] ||
+	       func_id == special_kfunc_list[KF_bpf_task_work_schedule_resume_impl];
 }
 
 static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
diff --git a/tools/testing/selftests/bpf/progs/file_reader.c b/tools/testing/selftests/bpf/progs/file_reader.c
index 2585f83b0ce5..eb8bdc51973d 100644
--- a/tools/testing/selftests/bpf/progs/file_reader.c
+++ b/tools/testing/selftests/bpf/progs/file_reader.c
@@ -77,7 +77,7 @@ int on_open_validate_file_read(void *c)
 		err = 1;
 		return 0;
 	}
-	bpf_task_work_schedule_signal(task, &work->tw, &arrmap, task_work_callback, NULL);
+	bpf_task_work_schedule_signal(task, &work->tw, &arrmap, task_work_callback);
 	return 0;
 }
 
diff --git a/tools/testing/selftests/bpf/progs/task_work.c b/tools/testing/selftests/bpf/progs/task_work.c
index 23217f06a3ec..eedd5c3dabb4 100644
--- a/tools/testing/selftests/bpf/progs/task_work.c
+++ b/tools/testing/selftests/bpf/progs/task_work.c
@@ -66,7 +66,7 @@ int oncpu_hash_map(struct pt_regs *args)
 	if (!work)
 		return 0;
 
-	bpf_task_work_schedule_resume(task, &work->tw, &hmap, process_work, NULL);
+	bpf_task_work_schedule_resume(task, &work->tw, &hmap, process_work);
 	return 0;
 }
 
@@ -80,7 +80,7 @@ int oncpu_array_map(struct pt_regs *args)
 	work = bpf_map_lookup_elem(&arrmap, &key);
 	if (!work)
 		return 0;
-	bpf_task_work_schedule_signal(task, &work->tw, &arrmap, process_work, NULL);
+	bpf_task_work_schedule_signal(task, &work->tw, &arrmap, process_work);
 	return 0;
 }
 
@@ -102,6 +102,6 @@ int oncpu_lru_map(struct pt_regs *args)
 	work = bpf_map_lookup_elem(&lrumap, &key);
 	if (!work || work->data[0])
 		return 0;
-	bpf_task_work_schedule_resume(task, &work->tw, &lrumap, process_work, NULL);
+	bpf_task_work_schedule_resume(task, &work->tw, &lrumap, process_work);
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/task_work_fail.c b/tools/testing/selftests/bpf/progs/task_work_fail.c
index 77fe8f28facd..82e4b8913333 100644
--- a/tools/testing/selftests/bpf/progs/task_work_fail.c
+++ b/tools/testing/selftests/bpf/progs/task_work_fail.c
@@ -53,7 +53,7 @@ int mismatch_map(struct pt_regs *args)
 	work = bpf_map_lookup_elem(&arrmap, &key);
 	if (!work)
 		return 0;
-	bpf_task_work_schedule_resume(task, &work->tw, &hmap, process_work, NULL);
+	bpf_task_work_schedule_resume(task, &work->tw, &hmap, process_work);
 	return 0;
 }
 
@@ -65,7 +65,7 @@ int no_map_task_work(struct pt_regs *args)
 	struct bpf_task_work tw;
 
 	task = bpf_get_current_task_btf();
-	bpf_task_work_schedule_resume(task, &tw, &hmap, process_work, NULL);
+	bpf_task_work_schedule_resume(task, &tw, &hmap, process_work);
 	return 0;
 }
 
@@ -76,7 +76,7 @@ int task_work_null(struct pt_regs *args)
 	struct task_struct *task;
 
 	task = bpf_get_current_task_btf();
-	bpf_task_work_schedule_resume(task, NULL, &hmap, process_work, NULL);
+	bpf_task_work_schedule_resume(task, NULL, &hmap, process_work);
 	return 0;
 }
 
@@ -91,6 +91,6 @@ int map_null(struct pt_regs *args)
 	work = bpf_map_lookup_elem(&arrmap, &key);
 	if (!work)
 		return 0;
-	bpf_task_work_schedule_resume(task, &work->tw, NULL, process_work, NULL);
+	bpf_task_work_schedule_resume(task, &work->tw, NULL, process_work);
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/task_work_stress.c b/tools/testing/selftests/bpf/progs/task_work_stress.c
index 90fca06fff56..1d4378f351ef 100644
--- a/tools/testing/selftests/bpf/progs/task_work_stress.c
+++ b/tools/testing/selftests/bpf/progs/task_work_stress.c
@@ -52,7 +52,7 @@ int schedule_task_work(void *ctx)
 			return 0;
 	}
 	err = bpf_task_work_schedule_signal(bpf_get_current_task_btf(), &work->tw, &hmap,
-					    process_work, NULL);
+					    process_work);
 	if (err)
 		__sync_fetch_and_add(&schedule_error, 1);
 	else
diff --git a/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c b/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c
index 96ff6749168b..a8696eb1febb 100644
--- a/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c
+++ b/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c
@@ -156,7 +156,7 @@ int task_work_non_sleepable_prog(void *ctx)
 	if (!task)
 		return 0;
 
-	bpf_task_work_schedule_resume(task, &val->tw, &task_work_map, task_work_cb, NULL);
+	bpf_task_work_schedule_resume(task, &val->tw, &task_work_map, task_work_cb);
 	return 0;
 }
 
@@ -176,6 +176,6 @@ int task_work_sleepable_prog(void *ctx)
 	if (!task)
 		return 0;
 
-	bpf_task_work_schedule_resume(task, &val->tw, &task_work_map, task_work_cb, NULL);
+	bpf_task_work_schedule_resume(task, &val->tw, &task_work_map, task_work_cb);
 	return 0;
 }
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH bpf-next v1 8/8] bpf: Re-define bpf_stream_vprintk as a magic kfunc
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
                   ` (6 preceding siblings ...)
  2025-10-29 19:01 ` [PATCH bpf-next v1 7/8] bpf: Re-define bpf_task_work_schedule_* kfuncs as magic Ihor Solodrai
@ 2025-10-29 19:01 ` Ihor Solodrai
  2025-10-30  0:44 ` [PATCH bpf-next v1 0/8] bpf: magic kernel functions Eduard Zingerman
  8 siblings, 0 replies; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:01 UTC (permalink / raw)
  To: bpf, andrii, ast; +Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team

* void *aux__prog => struct bpf_prog_aux *aux__magic
* Set KF_MAGIC_ARGS flag
* Add relevant symbols to magic_kfuncs list
* Update selftests to use the new signature

bpf_stream_vprintk macro is changed to use bpf_stream_vprintk_impl,
and the extern definition of bpf_stream_vprintk is replaced with _impl
version in bpf_helpers.h

This should help with backwards compatibility, as the API of
bpf_stream_vprintk macro hasn't changed.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 kernel/bpf/helpers.c                            | 2 +-
 kernel/bpf/stream.c                             | 9 ++++++---
 kernel/bpf/verifier.c                           | 2 ++
 tools/lib/bpf/bpf_helpers.h                     | 7 ++++---
 tools/testing/selftests/bpf/progs/stream_fail.c | 6 +++---
 5 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 6a095796433a..418c6b31ccc6 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4528,7 +4528,7 @@ BTF_ID_FLAGS(func, bpf_strncasestr);
 #if defined(CONFIG_BPF_LSM) && defined(CONFIG_CGROUPS)
 BTF_ID_FLAGS(func, bpf_cgroup_read_xattr, KF_RCU)
 #endif
-BTF_ID_FLAGS(func, bpf_stream_vprintk, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_stream_vprintk, KF_MAGIC_ARGS | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_task_work_schedule_signal, KF_MAGIC_ARGS | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_task_work_schedule_resume, KF_MAGIC_ARGS | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_dynptr_from_file, KF_TRUSTED_ARGS)
diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
index eb6c5a21c2ef..1a129fff765e 100644
--- a/kernel/bpf/stream.c
+++ b/kernel/bpf/stream.c
@@ -355,19 +355,22 @@ __bpf_kfunc_start_defs();
  * Avoid using enum bpf_stream_id so that kfunc users don't have to pull in the
  * enum in headers.
  */
-__bpf_kfunc int bpf_stream_vprintk(int stream_id, const char *fmt__str, const void *args, u32 len__sz, void *aux__prog)
+__bpf_kfunc int bpf_stream_vprintk(int stream_id,
+				   const char *fmt__str,
+				   const void *args,
+				   u32 len__sz,
+				   struct bpf_prog_aux *aux__magic)
 {
 	struct bpf_bprintf_data data = {
 		.get_bin_args	= true,
 		.get_buf	= true,
 	};
-	struct bpf_prog_aux *aux = aux__prog;
 	u32 fmt_size = strlen(fmt__str) + 1;
 	struct bpf_stream *stream;
 	u32 data_len = len__sz;
 	int ret, num_args;
 
-	stream = bpf_stream_get(stream_id, aux);
+	stream = bpf_stream_get(stream_id, aux__magic);
 	if (!stream)
 		return -ENOENT;
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ad4af5ddb523..9e38fbee9219 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3273,6 +3273,8 @@ BTF_ID(func, bpf_task_work_schedule_signal)
 BTF_ID(func, bpf_task_work_schedule_signal_impl)
 BTF_ID(func, bpf_task_work_schedule_resume)
 BTF_ID(func, bpf_task_work_schedule_resume_impl)
+BTF_ID(func, bpf_stream_vprintk)
+BTF_ID(func, bpf_stream_vprintk_impl)
 BTF_ID_LIST_END(magic_kfuncs)
 
 static s32 magic_kfunc_by_impl(s32 impl_func_id)
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 80c028540656..f41ca993c6d2 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -315,8 +315,9 @@ enum libbpf_tristate {
 			  ___param, sizeof(___param));		\
 })
 
-extern int bpf_stream_vprintk(int stream_id, const char *fmt__str, const void *args,
-			      __u32 len__sz, void *aux__prog) __weak __ksym;
+struct bpf_prog_aux;
+extern int bpf_stream_vprintk_impl(int stream_id, const char *fmt__str, const void *args,
+				   __u32 len__sz, struct bpf_prog_aux *aux__magic) __weak __ksym;
 
 #define bpf_stream_printk(stream_id, fmt, args...)				\
 ({										\
@@ -328,7 +329,7 @@ extern int bpf_stream_vprintk(int stream_id, const char *fmt__str, const void *a
 	___bpf_fill(___param, args);						\
 	_Pragma("GCC diagnostic pop")						\
 										\
-	bpf_stream_vprintk(stream_id, ___fmt, ___param, sizeof(___param), NULL);\
+	bpf_stream_vprintk_impl(stream_id, ___fmt, ___param, sizeof(___param), NULL);\
 })
 
 /* Use __bpf_printk when bpf_printk call has 3 or fewer fmt args
diff --git a/tools/testing/selftests/bpf/progs/stream_fail.c b/tools/testing/selftests/bpf/progs/stream_fail.c
index b4a0d0cc8ec8..8e8249f3521c 100644
--- a/tools/testing/selftests/bpf/progs/stream_fail.c
+++ b/tools/testing/selftests/bpf/progs/stream_fail.c
@@ -10,7 +10,7 @@ SEC("syscall")
 __failure __msg("Possibly NULL pointer passed")
 int stream_vprintk_null_arg(void *ctx)
 {
-	bpf_stream_vprintk(BPF_STDOUT, "", NULL, 0, NULL);
+	bpf_stream_vprintk(BPF_STDOUT, "", NULL, 0);
 	return 0;
 }
 
@@ -18,7 +18,7 @@ SEC("syscall")
 __failure __msg("R3 type=scalar expected=")
 int stream_vprintk_scalar_arg(void *ctx)
 {
-	bpf_stream_vprintk(BPF_STDOUT, "", (void *)46, 0, NULL);
+	bpf_stream_vprintk(BPF_STDOUT, "", (void *)46, 0);
 	return 0;
 }
 
@@ -26,7 +26,7 @@ SEC("syscall")
 __failure __msg("arg#1 doesn't point to a const string")
 int stream_vprintk_string_arg(void *ctx)
 {
-	bpf_stream_vprintk(BPF_STDOUT, ctx, NULL, 0, NULL);
+	bpf_stream_vprintk(BPF_STDOUT, ctx, NULL, 0);
 	return 0;
 }
 
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros
  2025-10-29 19:01 ` [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros Ihor Solodrai
@ 2025-10-29 19:41   ` bot+bpf-ci
  2025-10-29 20:44     ` Ihor Solodrai
  2025-10-29 23:54   ` Eduard Zingerman
  1 sibling, 1 reply; 36+ messages in thread
From: bot+bpf-ci @ 2025-10-29 19:41 UTC (permalink / raw)
  To: ihor.solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]


```
commit d616f53ce936875dd9f44cc77f9bed1198596463
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros

This commit adds two new preprocessor macros to btf_ids.h for
calculating the size of BTF_ID_LIST arrays using pointer arithmetic
between a list and its __end marker.

> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> index 139bdecec..27a4724d5 100644
> --- a/include/linux/btf_ids.h
> +++ b/include/linux/btf_ids.h
> @@ -97,6 +97,16 @@ asm(							\
>  __BTF_ID_LIST(name, local)				\
>  extern u32 name[];
>
> +/*
> + * The BTF_ID_LIST_END macro may be used to denote an end
> + * of a BTF_ID_LIST. This enables calculation of the list
> + * size with BTF_ID_LIST_SIZE.
> + */
> +#define BTF_ID_LIST_END(name) \
> +BTF_ID_LIST(name##__end)
> +#define BTF_ID_LIST_SIZE(name) \
> +(name##__end - name)
> +

Are the fallback definitions missing for CONFIG_DEBUG_INFO_BTF=n?

Looking at the #else branch starting at line 228, all existing macros
like BTF_ID_LIST, BTF_ID, BTF_SET_START, etc. have fallback definitions.
These new macros BTF_ID_LIST_END and BTF_ID_LIST_SIZE only exist in the
CONFIG_DEBUG_INFO_BTF=y path. When CONFIG_DEBUG_INFO_BTF is disabled,
code using these macros will fail to compile.

While there's no current usage that would break, the API is incomplete
compared to the established pattern in this header where every macro
works in both config states.

>  #define BTF_ID_LIST_GLOBAL(name, n)			\
>  __BTF_ID_LIST(name, globl)


```

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: `bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros`
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18919699520

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 19:01 ` [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS Ihor Solodrai
@ 2025-10-29 19:41   ` bot+bpf-ci
  2025-10-29 20:49     ` Ihor Solodrai
  2025-10-29 23:54   ` Eduard Zingerman
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 36+ messages in thread
From: bot+bpf-ci @ 2025-10-29 19:41 UTC (permalink / raw)
  To: ihor.solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]


```
commit 974828216758f5fff2d9783439b8daa20188ce8c
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

bpf: Support for kfuncs with KF_MAGIC_ARGS

This patch adds infrastructure for magic kfuncs, which have two BTF
representations: one for BPF programs and one for kernel implementation.
The verifier resolves these through a lookup table.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index cb1b483be..fcf0872b9 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3263,17 +3263,68 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
>  	return btf_vmlinux ?: ERR_PTR(-ENOENT);
>  }
>
> +/*
> + * magic_kfuncs is used as a list of (foo, foo_impl) pairs
> + */
> +BTF_ID_LIST(magic_kfuncs)
> +BTF_ID_UNUSED
> +BTF_ID_LIST_END(magic_kfuncs)
> +
> +static s32 magic_kfunc_by_impl(s32 impl_func_id)
> +{
> +	int i;
> +
> +	for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
> +		if (magic_kfuncs[i] == impl_func_id)
> +			return magic_kfuncs[i - 1];
> +	}
> +	return -ENOENT;
> +}
> +
> +static s32 impl_by_magic_kfunc(s32 func_id)
> +{
> +	int i;
> +
> +	for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
> +		if (magic_kfuncs[i] == func_id)
> +			return magic_kfuncs[i + 1];
                                    ^^^^^^^^^^^^^^^^^

Can impl_by_magic_kfunc() overflow magic_kfuncs[]? With the current
initialization using BTF_ID_UNUSED, BTF_ID_LIST_SIZE(magic_kfuncs)
equals 1. The loop condition checks i < 1, so when i=0 it executes and
accesses magic_kfuncs[i+1], which is magic_kfuncs[1]. This is outside
the array bounds.

The same issue applies generally: the loop checks i < size but accesses
i+1, so it should check i+1 < size instead.

> +	}
> +	return -ENOENT;
> +}

[ ... ]


```

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: `bpf: Support for kfuncs with KF_MAGIC_ARGS`
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18919699520

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros
  2025-10-29 19:41   ` bot+bpf-ci
@ 2025-10-29 20:44     ` Ihor Solodrai
  0 siblings, 0 replies; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 20:44 UTC (permalink / raw)
  To: bot+bpf-ci, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team, daniel,
	martin.lau, yonghong.song, clm

On 10/29/25 12:41 PM, bot+bpf-ci@kernel.org wrote:
> 
> ```
> commit d616f53ce936875dd9f44cc77f9bed1198596463
> Author: Ihor Solodrai <ihor.solodrai@linux.dev>
> 
> bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros
> 
> This commit adds two new preprocessor macros to btf_ids.h for
> calculating the size of BTF_ID_LIST arrays using pointer arithmetic
> between a list and its __end marker.
> 
>> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
>> index 139bdecec..27a4724d5 100644
>> --- a/include/linux/btf_ids.h
>> +++ b/include/linux/btf_ids.h
>> @@ -97,6 +97,16 @@ asm(							\
>>  __BTF_ID_LIST(name, local)				\
>>  extern u32 name[];
>>
>> +/*
>> + * The BTF_ID_LIST_END macro may be used to denote an end
>> + * of a BTF_ID_LIST. This enables calculation of the list
>> + * size with BTF_ID_LIST_SIZE.
>> + */
>> +#define BTF_ID_LIST_END(name) \
>> +BTF_ID_LIST(name##__end)
>> +#define BTF_ID_LIST_SIZE(name) \
>> +(name##__end - name)
>> +
> 
> Are the fallback definitions missing for CONFIG_DEBUG_INFO_BTF=n?

Indeed. Kernel build fails with CONFIG_DEBUG_INFO_BTF=n

Good bot.

> 
> Looking at the #else branch starting at line 228, all existing macros
> like BTF_ID_LIST, BTF_ID, BTF_SET_START, etc. have fallback definitions.
> These new macros BTF_ID_LIST_END and BTF_ID_LIST_SIZE only exist in the
> CONFIG_DEBUG_INFO_BTF=y path. When CONFIG_DEBUG_INFO_BTF is disabled,
> code using these macros will fail to compile.
> 
> While there's no current usage that would break, the API is incomplete
> compared to the established pattern in this header where every macro
> works in both config states.
> 
>>  #define BTF_ID_LIST_GLOBAL(name, n)			\
>>  __BTF_ID_LIST(name, globl)
> 
> 
> ```
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> In-Reply-To-Subject: `bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros`
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18919699520


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 19:41   ` bot+bpf-ci
@ 2025-10-29 20:49     ` Ihor Solodrai
  2025-10-29 23:59       ` Eduard Zingerman
  0 siblings, 1 reply; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-29 20:49 UTC (permalink / raw)
  To: bot+bpf-ci, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, eddyz87, tj, kernel-team, daniel,
	martin.lau, yonghong.song, clm

On 10/29/25 12:41 PM, bot+bpf-ci@kernel.org wrote:
> 
> ```
> commit 974828216758f5fff2d9783439b8daa20188ce8c
> Author: Ihor Solodrai <ihor.solodrai@linux.dev>
> 
> bpf: Support for kfuncs with KF_MAGIC_ARGS
> 
> This patch adds infrastructure for magic kfuncs, which have two BTF
> representations: one for BPF programs and one for kernel implementation.
> The verifier resolves these through a lookup table.
> 
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index cb1b483be..fcf0872b9 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -3263,17 +3263,68 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
>>  	return btf_vmlinux ?: ERR_PTR(-ENOENT);
>>  }
>>
>> +/*
>> + * magic_kfuncs is used as a list of (foo, foo_impl) pairs
>> + */
>> +BTF_ID_LIST(magic_kfuncs)
>> +BTF_ID_UNUSED
>> +BTF_ID_LIST_END(magic_kfuncs)
>> +
>> +static s32 magic_kfunc_by_impl(s32 impl_func_id)
>> +{
>> +	int i;
>> +
>> +	for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
>> +		if (magic_kfuncs[i] == impl_func_id)
>> +			return magic_kfuncs[i - 1];
>> +	}
>> +	return -ENOENT;
>> +}
>> +
>> +static s32 impl_by_magic_kfunc(s32 func_id)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
>> +		if (magic_kfuncs[i] == func_id)
>> +			return magic_kfuncs[i + 1];
>                                     ^^^^^^^^^^^^^^^^^
> 
> Can impl_by_magic_kfunc() overflow magic_kfuncs[]? With the current
> initialization using BTF_ID_UNUSED, BTF_ID_LIST_SIZE(magic_kfuncs)
> equals 1. The loop condition checks i < 1, so when i=0 it executes and
> accesses magic_kfuncs[i+1], which is magic_kfuncs[1]. This is outside
> the array bounds.

Hmm... Given we do i += 2, this can't happen if magic_kfuncs table is
defined correctly. Also if BTF_ID_UNUSED is passed in here, we have
bigger problems.

I guess changing the loop condition to size-1 wouldn't hurt.

> 
> The same issue applies generally: the loop checks i < size but accesses
> i+1, so it should check i+1 < size instead.
> 
>> +	}
>> +	return -ENOENT;
>> +}
> 
> [ ... ]
> 
> 
> ```
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> In-Reply-To-Subject: `bpf: Support for kfuncs with KF_MAGIC_ARGS`
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18919699520


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 19:01 ` [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS Ihor Solodrai
  2025-10-29 19:41   ` bot+bpf-ci
@ 2025-10-29 23:54   ` Eduard Zingerman
  2025-10-30  0:03     ` Alexei Starovoitov
  2025-10-30 16:31     ` Ihor Solodrai
  2025-10-30 10:24   ` kernel test robot
                     ` (2 subsequent siblings)
  4 siblings, 2 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-29 23:54 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> A kernel function bpf_foo with KF_MAGIC_ARGS flag is expected to have
                                 ^^^^^^^^^^^^^
		I don't like this name very much.
		It bears very little context.
		Imo, KF_IMPLICIT_ARGS fits the use case much better.

> two types in BTF:
>   * `bpf_foo` with a function prototype that omits __magic arguments
>   * `bpf_foo_impl` with a function prototype that matches kernel
>      declaration, but doesn't have a ksym associated with its name

Could you please start with an example here?
Stating how `bpf_foo` needs to be declared in kernel, and what are the
options to invoke it from bpf.  Then proceed with BTF details, etc.

> In order to support magic kfuncs the verifier has to know how to
> resolve calls both of `bpf_foo` and `bpf_foo_impl` to the correct BTF
> function prototype and address.
> 
> In add_kfunc_call() kfunc flags are inspected to detect a magic kfunc
> or its _impl, and then the address and func_proto are adjusted for the
> kfunc descriptor.
> 
> In fetch_kfunc_meta() similar logic is used to fixup the contents of
> struct bpf_kfunc_call_arg_meta.
> 
> In check_kfunc_call() reset the subreg_def of registers holding magic
> arguments to correctly track zero extensions.
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---
>  include/linux/btf.h   |   1 +
>  kernel/bpf/verifier.c | 123 ++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 120 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 9c64bc5e5789..3fe20514692f 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -79,6 +79,7 @@
>  #define KF_ARENA_RET    (1 << 13) /* kfunc returns an arena pointer */
>  #define KF_ARENA_ARG1   (1 << 14) /* kfunc takes an arena pointer as its first argument */
>  #define KF_ARENA_ARG2   (1 << 15) /* kfunc takes an arena pointer as its second argument */
> +#define KF_MAGIC_ARGS   (1 << 16) /* kfunc signature is different from its BPF signature */
>  
>  /*
>   * Tag marking a kernel function as a kfunc. This is meant to minimize the
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index cb1b483be0fa..fcf0872b9e3d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3263,17 +3263,68 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
>  	return btf_vmlinux ?: ERR_PTR(-ENOENT);
>  }
>  
> +/*
> + * magic_kfuncs is used as a list of (foo, foo_impl) pairs
> + */
> +BTF_ID_LIST(magic_kfuncs)
> +BTF_ID_UNUSED
> +BTF_ID_LIST_END(magic_kfuncs)
> +
> +static s32 magic_kfunc_by_impl(s32 impl_func_id)
> +{
> +	int i;
> +
> +	for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
> +		if (magic_kfuncs[i] == impl_func_id)
                                       ^^^^^
Nit: similarly, I'd rename this to something like "implicit_func_id"
     or "fake_func_id. "impl" is confusing because this id has nothing
     to do with implementation.

> +			return magic_kfuncs[i - 1];
> +	}
> +	return -ENOENT;
> +}
> +
> +static s32 impl_by_magic_kfunc(s32 func_id)
> +{
> +	int i;
> +
> +	for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
> +		if (magic_kfuncs[i] == func_id)
> +			return magic_kfuncs[i + 1];
> +	}
> +	return -ENOENT;
> +}
> +
> +static const struct btf_type *find_magic_kfunc_proto(struct btf *desc_btf, s32 func_id)
> +{
> +	const struct btf_type *impl_func, *func_proto;
> +	u32 impl_func_id;
> +
> +	impl_func_id = impl_by_magic_kfunc(func_id);
> +	if (impl_func_id < 0)
> +		return NULL;
> +
> +	impl_func = btf_type_by_id(desc_btf, impl_func_id);
> +	if (!impl_func || !btf_type_is_func(impl_func))
> +		return NULL;
> +
> +	func_proto = btf_type_by_id(desc_btf, impl_func->type);
> +	if (!func_proto || !btf_type_is_func_proto(func_proto))
> +		return NULL;
> +
> +	return func_proto;
> +}
> +
>  static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
>  {
> -	const struct btf_type *func, *func_proto;
> +	const struct btf_type *func, *func_proto, *tmp_func;
>  	struct bpf_kfunc_btf_tab *btf_tab;
> +	const char *func_name, *tmp_name;
>  	struct btf_func_model func_model;
>  	struct bpf_kfunc_desc_tab *tab;
>  	struct bpf_prog_aux *prog_aux;
>  	struct bpf_kfunc_desc *desc;
> -	const char *func_name;
>  	struct btf *desc_btf;
>  	unsigned long addr;
> +	u32 *kfunc_flags;
> +	s32 tmp_func_id;
>  	int err;
>  
>  	prog_aux = env->prog->aux;
> @@ -3349,8 +3400,37 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
>  		return -EINVAL;
>  	}
>  
> +	kfunc_flags = btf_kfunc_flags(desc_btf, func_id, env->prog);
>  	func_name = btf_name_by_offset(desc_btf, func->name_off);
>  	addr = kallsyms_lookup_name(func_name);
> +
> +	/* This may be an _impl kfunc with KF_MAGIC_ARGS counterpart */
> +	if (unlikely(!addr && !kfunc_flags)) {
> +		tmp_func_id = magic_kfunc_by_impl(func_id);

I think there is no need to hide magic_kfunc_by_impl() call behind the
above condition. It can be moved before kfunc_flags assignment.
Then it wont be necessary to textually repeat btf_name_by_offset() and
kallsyms_lookup_name() calls.

> +		if (tmp_func_id < 0)
> +			return -EACCES;

Nit: this skips proper error reporting: "cannot find address for kernel function %s\n".

> +		tmp_func = btf_type_by_id(desc_btf, tmp_func_id);
> +		if (!tmp_func || !btf_type_is_func(tmp_func))

Nit: this condition indicates a verifier bug, should it be reported as such?

> +			return -EACCES;
> +		tmp_name = btf_name_by_offset(desc_btf, tmp_func->name_off);
> +		addr = kallsyms_lookup_name(tmp_name);
> +	}
> +
> +	/*
> +	 * Note that kfunc_flags may be NULL at this point, which means that we couldn't find
> +	 * func_id in any relevant kfunc_id_set. This most likely indicates an invalid kfunc call.
> +	 * However we don't want to fail the verification here, because invalid calls may be
> +	 * eliminated as dead code later.
> +	 */
> +	if (unlikely(kfunc_flags && KF_MAGIC_ARGS & *kfunc_flags)) {
> +		func_proto = find_magic_kfunc_proto(desc_btf, func_id);
> +		if (!func_proto) {
> +			verbose(env, "cannot find _impl proto for kernel function %s\n",
> +			func_name);
> +			return -EINVAL;
> +		}
> +	}
> +
>  	if (!addr) {
>  		verbose(env, "cannot find address for kernel function %s\n",
>  			func_name);
> @@ -12051,6 +12131,11 @@ static bool is_kfunc_arg_irq_flag(const struct btf *btf, const struct btf_param
>  	return btf_param_match_suffix(btf, arg, "__irq_flag");
>  }
>  
> +static bool is_kfunc_arg_magic(const struct btf *btf, const struct btf_param *arg)
> +{
> +	return btf_param_match_suffix(btf, arg, "__magic");
> +}
> +
>  static bool is_kfunc_arg_prog(const struct btf *btf, const struct btf_param *arg)
>  {
>  	return btf_param_match_suffix(btf, arg, "__prog");
> @@ -13613,6 +13698,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
>  	u32 func_id, *kfunc_flags;
>  	const char *func_name;
>  	struct btf *desc_btf;
> +	s32 tmp_func_id;
>  
>  	if (kfunc_name)
>  		*kfunc_name = NULL;
> @@ -13632,10 +13718,28 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
>  	func_proto = btf_type_by_id(desc_btf, func->type);
>  
>  	kfunc_flags = btf_kfunc_flags_if_allowed(desc_btf, func_id, env->prog);
> -	if (!kfunc_flags) {
> -		return -EACCES;
> +	if (unlikely(!kfunc_flags)) {

What if we patch insn->imm to use the "fake" function id in add_kfunc_call()?
Then modifications to fetch_kfunc_meta() wont be necessary.

> +		/*
> +		 * An _impl kfunc with KF_MAGIC_ARGS counterpart
> +		 * does not have its own kfunc flags.
> +		 */
> +		tmp_func_id = magic_kfunc_by_impl(func_id);
> +		if (tmp_func_id < 0)
> +			return -EACCES;
> +		kfunc_flags = btf_kfunc_flags_if_allowed(desc_btf, tmp_func_id, env->prog);
> +		if (!kfunc_flags)
> +			return -EACCES;
> +	} else if (unlikely(KF_MAGIC_ARGS & *kfunc_flags)) {
> +		/*
> +		 * An actual func_proto of a kfunc with KF_MAGIC_ARGS flag
> +		 * can be found through the corresponding _impl kfunc.
> +		 */
> +		func_proto = find_magic_kfunc_proto(desc_btf, func_id);
>  	}
>  
> +	if (!func_proto)
> +		return -EACCES;
> +
>  	memset(meta, 0, sizeof(*meta));
>  	meta->btf = desc_btf;
>  	meta->func_id = func_id;
> @@ -14189,6 +14293,17 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	for (i = 0; i < nargs; i++) {
>  		u32 regno = i + 1;
>  
> +		/*
> +		 * Magic arguments are set after main verification pass.
> +		 * For correct tracking of zero-extensions we have to reset subreg_def for such
> +		 * args. Otherwise mark_btf_func_reg_size() will be inspecting subreg_def of regs
> +		 * from an earlier (irrelevant) point in the program, which may lead to an error
> +		 * in opt_subreg_zext_lo32_rnd_hi32().
> +		 */
> +		if (unlikely(KF_MAGIC_ARGS & meta.kfunc_flags
> +				&& is_kfunc_arg_magic(desc_btf, &args[i])))
> +			regs[regno].subreg_def = DEF_NOT_SUBREG;
> +
>  		t = btf_type_skip_modifiers(desc_btf, args[i].type, NULL);
>  		if (btf_type_is_ptr(t))
>  			mark_btf_func_reg_size(env, regno, sizeof(void *));

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros
  2025-10-29 19:01 ` [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros Ihor Solodrai
  2025-10-29 19:41   ` bot+bpf-ci
@ 2025-10-29 23:54   ` Eduard Zingerman
  1 sibling, 0 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-29 23:54 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> Implement macros in btf_ids.h to enable a calculation of BTF_ID_LIST
> size. This is done by declaring an additional __end symbol which can
> then be used as an indicator of the end of an array.
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---
>  include/linux/btf_ids.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> index 139bdececdcf..27a4724d5aa9 100644
> --- a/include/linux/btf_ids.h
> +++ b/include/linux/btf_ids.h
> @@ -97,6 +97,16 @@ asm(							\
>  __BTF_ID_LIST(name, local)				\
>  extern u32 name[];
>  
> +/*
> + * The BTF_ID_LIST_END macro may be used to denote an end
> + * of a BTF_ID_LIST. This enables calculation of the list
> + * size with BTF_ID_LIST_SIZE.
> + */
> +#define BTF_ID_LIST_END(name) \
> +BTF_ID_LIST(name##__end)
> +#define BTF_ID_LIST_SIZE(name) \
> +(name##__end - name)
> +

Nit: no need for line break?

  #define BTF_ID_LIST_SIZE(name) (name##__end - name)

>  #define BTF_ID_LIST_GLOBAL(name, n)			\
>  __BTF_ID_LIST(name, globl)
>  


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 2/8] bpf: Refactor btf_kfunc_id_set_contains
  2025-10-29 19:01 ` [PATCH bpf-next v1 2/8] bpf: Refactor btf_kfunc_id_set_contains Ihor Solodrai
@ 2025-10-29 23:55   ` Eduard Zingerman
  0 siblings, 0 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-29 23:55 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> btf_kfunc_id_set_contains() is called by fetch_kfunc_meta() in the BPF
> verifier to get the kfunc flags stored in the .BTF_ids ELF section.
> If it returns NULL instead of a valid pointer, it's interpreted by the
> verifier as an illegal kfunc usage which fails the verification.
> 
> Conceptually, there are two potential reasons for
> btf_kfunc_id_set_contains() to return NULL:
> 
>   1. Provided kfunc BTF id is not present in relevant kfunc id sets.
>   2. The kfunc is not allowed, as determined by the program type
>      specific filter [1].
> 
> The filter functions accept a pointer to `struct bpf_prog`, so they
> might implicitly depend on earlier stages of verification, when
> bpf_prog members are set.
> 
> For example, bpf_qdisc_kfunc_filter() in linux/net/sched/bpf_qdisc.c
> inspects prog->aux->st_ops [2], which is initialized in:
> 
>     check_attach_btf_id() -> check_struct_ops_btf_id()
> 
> So far this hasn't been an issue, because fetch_kfunc_meta() is the
> only place where lookup + filter logic is applied to a kfunc id.
> 
> However in subsequent patches of this series it is necessary to
> inspect kfunc flags earlier in BPF verifier, in the add_kfunc_call().
> 
> To resolve this, refactor btf_kfunc_id_set_contains() into two
> interface functions: btf_kfunc_flags() that does not apply the
> filters, and btf_kfunc_flags_if_allowed() that does.
> 
> [1] https://lore.kernel.org/all/20230519225157.760788-7-aditi.ghag@isovalent.com/
> [2] https://lore.kernel.org/all/20250409214606.2000194-4-ameryhung@gmail.com/
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---

This preserves original semantics, as far as I can tell,
although a more logical split for API functions seem to be:
- btf_kfunc_is_allowed()
- btf_kfunc_flags()

Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>

[...]


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 20:49     ` Ihor Solodrai
@ 2025-10-29 23:59       ` Eduard Zingerman
  0 siblings, 0 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-29 23:59 UTC (permalink / raw)
  To: Ihor Solodrai, bot+bpf-ci, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team, daniel, martin.lau,
	yonghong.song, clm

On Wed, 2025-10-29 at 13:49 -0700, Ihor Solodrai wrote:

[...]

> > > +static s32 impl_by_magic_kfunc(s32 func_id)
> > > +{
> > > +	int i;
> > > +
> > > +	for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
> > > +		if (magic_kfuncs[i] == func_id)
> > > +			return magic_kfuncs[i + 1];
> >                                     ^^^^^^^^^^^^^^^^^
> > 
> > Can impl_by_magic_kfunc() overflow magic_kfuncs[]? With the current
> > initialization using BTF_ID_UNUSED, BTF_ID_LIST_SIZE(magic_kfuncs)
> > equals 1. The loop condition checks i < 1, so when i=0 it executes and
> > accesses magic_kfuncs[i+1], which is magic_kfuncs[1]. This is outside
> > the array bounds.
> 
> Hmm... Given we do i += 2, this can't happen if magic_kfuncs table is
> defined correctly. Also if BTF_ID_UNUSED is passed in here, we have
> bigger problems.
> 
> I guess changing the loop condition to size-1 wouldn't hurt.

The code is fine and there is no need to bow to the AI overlord.
That time will come, but it hasn't come yet.

[...]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 23:54   ` Eduard Zingerman
@ 2025-10-30  0:03     ` Alexei Starovoitov
  2025-10-30 16:31     ` Ihor Solodrai
  1 sibling, 0 replies; 36+ messages in thread
From: Alexei Starovoitov @ 2025-10-30  0:03 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Ihor Solodrai, bpf, Andrii Nakryiko, Alexei Starovoitov, dwarves,
	Alan Maguire, Arnaldo Carvalho de Melo, Tejun Heo, Kernel Team

On Wed, Oct 29, 2025 at 4:54 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> > A kernel function bpf_foo with KF_MAGIC_ARGS flag is expected to have
>                                  ^^^^^^^^^^^^^
>                 I don't like this name very much.
>                 It bears very little context.
>                 Imo, KF_IMPLICIT_ARGS fits the use case much better.

+1
I hate the name as well.
KF_IMPLICIT_ARGS sounds much better and can be abbreviated
to "__impl" in arg names.
That way it will accidentally match our existing _impl() kfuncs
though there _impl meant "implementation".

I think this double meaning of "impl" as "implementation specific"
and "implicit" actually fits.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 5/8] bpf: Re-define bpf_wq_set_callback as magic kfunc
  2025-10-29 19:01 ` [PATCH bpf-next v1 5/8] bpf: Re-define bpf_wq_set_callback as magic kfunc Ihor Solodrai
@ 2025-10-30  0:16   ` Eduard Zingerman
  0 siblings, 0 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-30  0:16 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:

[...]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 67914464d503..3c9e963d879b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[...]

> @@ -12916,9 +12919,10 @@ static bool is_bpf_throw_kfunc(struct bpf_insn *insn)
>  	       insn->imm == special_kfunc_list[KF_bpf_throw];
>  }
>  
> -static bool is_bpf_wq_set_callback_impl_kfunc(u32 btf_id)
> +static bool is_bpf_wq_set_callback_kfunc(u32 btf_id)
>  {
> -	return btf_id == special_kfunc_list[KF_bpf_wq_set_callback_impl];
> +	return btf_id == special_kfunc_list[KF_bpf_wq_set_callback_impl] ||
> +	       btf_id == special_kfunc_list[KF_bpf_wq_set_callback];

If insn->imm is patched in add_kfunc_call() there will be no need to
handle two cases here.

>  }
>  
>  static bool is_callback_calling_kfunc(u32 btf_id)
> @@ -14035,7 +14039,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  		meta.r0_rdonly = false;
>  	}
>  
> -	if (is_bpf_wq_set_callback_impl_kfunc(meta.func_id)) {
> +	if (is_bpf_wq_set_callback_kfunc(meta.func_id)) {
>  		err = push_callback_call(env, insn, insn_idx, meta.subprogno,
>  					 set_timer_callback_state);
>  		if (err) {
> diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
> index 2cd9165c7348..68a49b1f77ae 100644
> --- a/tools/testing/selftests/bpf/bpf_experimental.h
> +++ b/tools/testing/selftests/bpf/bpf_experimental.h

Nit: The tests won't fail with the above changes, right?
     If so, we usually ship changes to selftests in separate patches.

> @@ -580,11 +580,6 @@ extern void bpf_iter_css_destroy(struct bpf_iter_css *it) __weak __ksym;
>  
>  extern int bpf_wq_init(struct bpf_wq *wq, void *p__map, unsigned int flags) __weak __ksym;
>  extern int bpf_wq_start(struct bpf_wq *wq, unsigned int flags) __weak __ksym;
> -extern int bpf_wq_set_callback_impl(struct bpf_wq *wq,
> -		int (callback_fn)(void *map, int *key, void *value),
> -		unsigned int flags__k, void *aux__ign) __ksym;
> -#define bpf_wq_set_callback(timer, cb, flags) \
> -	bpf_wq_set_callback_impl(timer, cb, flags, NULL)

...

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 6/8] bpf,docs: Document KF_MAGIC_ARGS flag and __magic annotation
  2025-10-29 19:01 ` [PATCH bpf-next v1 6/8] bpf,docs: Document KF_MAGIC_ARGS flag and __magic annotation Ihor Solodrai
@ 2025-10-30  0:21   ` Eduard Zingerman
  0 siblings, 0 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-30  0:21 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> Add sections explaining KF_MAGIC_ARGS kfunc flag and __magic argument
> annotation. Mark __prog annotation as deprecated.
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---
>  Documentation/bpf/kfuncs.rst | 49 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
> index e38941370b90..1a261f84e157 100644
> --- a/Documentation/bpf/kfuncs.rst
> +++ b/Documentation/bpf/kfuncs.rst
> @@ -160,7 +160,7 @@ Or::
>                  ...
>          }
>  
> -2.2.6 __prog Annotation
> +2.2.6 __prog Annotation (deprecated, use __magic instead)
>  ---------------------------

I don't see any kfuncs that use __prog parameter suffix after your
changes, this section can be dropped altogether.

>  This annotation is used to indicate that the argument needs to be fixed up to
>  the bpf_prog_aux of the caller BPF program. Any value passed into this argument

[...]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
                   ` (7 preceding siblings ...)
  2025-10-29 19:01 ` [PATCH bpf-next v1 8/8] bpf: Re-define bpf_stream_vprintk as a magic kfunc Ihor Solodrai
@ 2025-10-30  0:44 ` Eduard Zingerman
  2025-10-30  6:11   ` Eduard Zingerman
  8 siblings, 1 reply; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-30  0:44 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:

Do we break compatibility with old pahole versions after this
patch-set? Old paholes won't synthesize the _impl kfuncs, so:
- binary compatibility between new-kernel/old-pahole + old-bpf
  will be broken, as there would be no _impl kfuncs;
- new-kernel/old-pahole + new-bpf won't work either, as kernel will be
  unable to find non-_impl function names for existing kfuncs.

[...]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30  0:44 ` [PATCH bpf-next v1 0/8] bpf: magic kernel functions Eduard Zingerman
@ 2025-10-30  6:11   ` Eduard Zingerman
  2025-10-30 18:14     ` Eduard Zingerman
  0 siblings, 1 reply; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-30  6:11 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> 
> Do we break compatibility with old pahole versions after this
> patch-set? Old paholes won't synthesize the _impl kfuncs, so:
> - binary compatibility between new-kernel/old-pahole + old-bpf
>   will be broken, as there would be no _impl kfuncs;
> - new-kernel/old-pahole + new-bpf won't work either, as kernel will
> be
>   unable to find non-_impl function names for existing kfuncs.
> 
> [...]

Point being, if we are going to break backwards compatibility the
following things need an update:
- Documentation/process/changes.rst
  minimal pahole version has to be bumped
- scripts/Makefile.btf
  All the different flags and options for different pahole
  versions can be dropped.

---

On the other hand, I'm not sure this useful but relatively obscure
feature grants such a compatibility break. Some time ago Ihor
advocated for just having two functions in the kernel, so that BTF
will be generated for both. And I think that someone suggested putting
the fake function to a discard-able section.
This way the whole thing can be done in kernel only.
E.g. it will look like so:

  __bpf_kfunc void btf_foo_impl(struct bpf_prog_aux p__implicit)
  { /* real impl here */ }

  __bpf_kfunc_proto void btf_foo(void) {}

Assuming that __bpf_kfunc_proto hides all the necessary attributes.
Not much boilerplate, and a tad easier to understand where second
prototype comes from, no need to read pahole.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 19:01 ` [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS Ihor Solodrai
  2025-10-29 19:41   ` bot+bpf-ci
  2025-10-29 23:54   ` Eduard Zingerman
@ 2025-10-30 10:24   ` kernel test robot
  2025-10-30 11:58   ` kernel test robot
  2025-10-30 13:54   ` kernel test robot
  4 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2025-10-30 10:24 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: oe-kbuild-all, dwarves, alan.maguire, acme, eddyz87, tj,
	kernel-team

Hi Ihor,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Ihor-Solodrai/bpf-Add-BTF_ID_LIST_END-and-BTF_ID_LIST_SIZE-macros/20251030-030608
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20251029190113.3323406-4-ihor.solodrai%40linux.dev
patch subject: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
config: x86_64-buildonly-randconfig-003-20251030 (https://download.01.org/0day-ci/archive/20251030/202510301811.fP0doUEk-lkp@intel.com/config)
compiler: gcc-13 (Debian 13.3.0-16) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251030/202510301811.fP0doUEk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510301811.fP0doUEk-lkp@intel.com/

All errors (new ones prefixed by >>):

         |                       ^
   include/linux/compiler.h:166:29: note: in expansion of macro '__PASTE'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:166:37: note: in expansion of macro '__PASTE'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/compiler.h:286:9: note: in expansion of macro '__UNIQUE_ID'
     286 |         __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
         |         ^~~~~~~~~~~
   include/linux/compiler.h:289:9: note: in expansion of macro '___ADDRESSABLE'
     289 |         ___ADDRESSABLE(sym, __section(".discard.addressable"))
         |         ^~~~~~~~~~~~~~
   include/linux/init.h:250:9: note: in expansion of macro '__ADDRESSABLE'
     250 |         __ADDRESSABLE(fn)
         |         ^~~~~~~~~~~~~
   include/linux/init.h:255:9: note: in expansion of macro '__define_initcall_stub'
     255 |         __define_initcall_stub(__stub, fn)                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/init.h:268:9: note: in expansion of macro '____define_initcall'
     268 |         ____define_initcall(fn,                                 \
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/init.h:274:9: note: in expansion of macro '__unique_initcall'
     274 |         __unique_initcall(fn, id, __sec, __initcall_id(fn))
         |         ^~~~~~~~~~~~~~~~~
   include/linux/init.h:276:35: note: in expansion of macro '___define_initcall'
     276 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
         |                                   ^~~~~~~~~~~~~~~~~~
   include/linux/init.h:307:41: note: in expansion of macro '__define_initcall'
     307 | #define late_initcall(fn)               __define_initcall(fn, 7)
         |                                         ^~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:18983:1: note: in expansion of macro 'late_initcall'
   18983 | late_initcall(unbound_reg_init);
         | ^~~~~~~~~~~~~
   In file included from include/uapi/linux/filter.h:9,
                    from include/linux/bpf.h:8:
   kernel/bpf/verifier.c:18983:15: error: 'unbound_reg_init' undeclared (first use in this function); did you mean 'unbound_reg'?
   18983 | late_initcall(unbound_reg_init);
         |               ^~~~~~~~~~~~~~~~
   include/linux/compiler.h:286:72: note: in definition of macro '___ADDRESSABLE'
     286 |         __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
         |                                                                        ^~~
   include/linux/init.h:250:9: note: in expansion of macro '__ADDRESSABLE'
     250 |         __ADDRESSABLE(fn)
         |         ^~~~~~~~~~~~~
   include/linux/init.h:255:9: note: in expansion of macro '__define_initcall_stub'
     255 |         __define_initcall_stub(__stub, fn)                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/init.h:268:9: note: in expansion of macro '____define_initcall'
     268 |         ____define_initcall(fn,                                 \
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/init.h:274:9: note: in expansion of macro '__unique_initcall'
     274 |         __unique_initcall(fn, id, __sec, __initcall_id(fn))
         |         ^~~~~~~~~~~~~~~~~
   include/linux/init.h:276:35: note: in expansion of macro '___define_initcall'
     276 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
         |                                   ^~~~~~~~~~~~~~~~~~
   include/linux/init.h:307:41: note: in expansion of macro '__define_initcall'
     307 | #define late_initcall(fn)               __define_initcall(fn, 7)
         |                                         ^~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:18983:1: note: in expansion of macro 'late_initcall'
   18983 | late_initcall(unbound_reg_init);
         | ^~~~~~~~~~~~~
   kernel/bpf/verifier.c:18983:15: note: each undeclared identifier is reported only once for each function it appears in
   18983 | late_initcall(unbound_reg_init);
         |               ^~~~~~~~~~~~~~~~
   include/linux/compiler.h:286:72: note: in definition of macro '___ADDRESSABLE'
     286 |         __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
         |                                                                        ^~~
   include/linux/init.h:250:9: note: in expansion of macro '__ADDRESSABLE'
     250 |         __ADDRESSABLE(fn)
         |         ^~~~~~~~~~~~~
   include/linux/init.h:255:9: note: in expansion of macro '__define_initcall_stub'
     255 |         __define_initcall_stub(__stub, fn)                      \
         |         ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/init.h:268:9: note: in expansion of macro '____define_initcall'
     268 |         ____define_initcall(fn,                                 \
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/init.h:274:9: note: in expansion of macro '__unique_initcall'
     274 |         __unique_initcall(fn, id, __sec, __initcall_id(fn))
         |         ^~~~~~~~~~~~~~~~~
   include/linux/init.h:276:35: note: in expansion of macro '___define_initcall'
     276 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
         |                                   ^~~~~~~~~~~~~~~~~~
   include/linux/init.h:307:41: note: in expansion of macro '__define_initcall'
     307 | #define late_initcall(fn)               __define_initcall(fn, 7)
         |                                         ^~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:18983:1: note: in expansion of macro 'late_initcall'
   18983 | late_initcall(unbound_reg_init);
         | ^~~~~~~~~~~~~
   In file included from include/linux/printk.h:6,
                    from include/asm-generic/bug.h:22,
                    from arch/x86/include/asm/bug.h:108,
                    from include/linux/bug.h:5,
                    from include/linux/alloc_tag.h:8,
                    from include/linux/workqueue.h:9,
                    from include/linux/bpf.h:11:
>> include/linux/init.h:256:9: error: expected declaration specifiers before 'asm'
     256 |         asm(".section   \"" __sec "\", \"a\"            \n"     \
         |         ^~~
   include/linux/init.h:268:9: note: in expansion of macro '____define_initcall'
     268 |         ____define_initcall(fn,                                 \
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/init.h:274:9: note: in expansion of macro '__unique_initcall'
     274 |         __unique_initcall(fn, id, __sec, __initcall_id(fn))
         |         ^~~~~~~~~~~~~~~~~
   include/linux/init.h:276:35: note: in expansion of macro '___define_initcall'
     276 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
         |                                   ^~~~~~~~~~~~~~~~~~
   include/linux/init.h:307:41: note: in expansion of macro '__define_initcall'
     307 | #define late_initcall(fn)               __define_initcall(fn, 7)
         |                                         ^~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:18983:1: note: in expansion of macro 'late_initcall'
   18983 | late_initcall(unbound_reg_init);
         | ^~~~~~~~~~~~~
   In file included from include/linux/init.h:5:
   include/linux/build_bug.h:78:41: error: expected declaration specifiers before '_Static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                         ^~~~~~~~~~~~~~
   include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert'
      77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
         |                                  ^~~~~~~~~~~~~~~
   include/linux/init.h:260:9: note: in expansion of macro 'static_assert'
     260 |         static_assert(__same_type(initcall_t, &fn));
         |         ^~~~~~~~~~~~~
   include/linux/init.h:268:9: note: in expansion of macro '____define_initcall'
     268 |         ____define_initcall(fn,                                 \
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/init.h:274:9: note: in expansion of macro '__unique_initcall'
     274 |         __unique_initcall(fn, id, __sec, __initcall_id(fn))
         |         ^~~~~~~~~~~~~~~~~
   include/linux/init.h:276:35: note: in expansion of macro '___define_initcall'
     276 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
         |                                   ^~~~~~~~~~~~~~~~~~
   include/linux/init.h:307:41: note: in expansion of macro '__define_initcall'
     307 | #define late_initcall(fn)               __define_initcall(fn, 7)
         |                                         ^~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:18983:1: note: in expansion of macro 'late_initcall'
   18983 | late_initcall(unbound_reg_init);
         | ^~~~~~~~~~~~~
   kernel/bpf/verifier.c:18983:32: error: expected declaration specifiers before ';' token
   18983 | late_initcall(unbound_reg_init);
         |                                ^
   kernel/bpf/verifier.c:18987:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   18987 | {
         | ^
   kernel/bpf/verifier.c:19002:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19002 | {
         | ^
   kernel/bpf/verifier.c:19015:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19015 | {
         | ^
   kernel/bpf/verifier.c:19139:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19139 | {
         | ^
   kernel/bpf/verifier.c:19212:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19212 | {
         | ^
   kernel/bpf/verifier.c:19232:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19232 | {
         | ^
   kernel/bpf/verifier.c:19241:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19241 | {
         | ^
   kernel/bpf/verifier.c:19282:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19282 | {
         | ^
   kernel/bpf/verifier.c:19343:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19343 | {
         | ^
   kernel/bpf/verifier.c:19373:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19373 | {
         | ^
   kernel/bpf/verifier.c:19390:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19390 | {
         | ^
   kernel/bpf/verifier.c:19453:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19453 | {
         | ^
   kernel/bpf/verifier.c:19477:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19477 | {
         | ^
   kernel/bpf/verifier.c:19833:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19833 | {
         | ^
   kernel/bpf/verifier.c:19861:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19861 | {
         | ^
   kernel/bpf/verifier.c:19867:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19867 | {
         | ^
   kernel/bpf/verifier.c:19878:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19878 | {
         | ^
   kernel/bpf/verifier.c:19884:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
   19884 | {
         | ^
   kernel/bpf/verifier.c:19930:1: warning: empty declaration


vim +/asm +256 include/linux/init.h

a8cccdd954732a5 Sami Tolvanen  2020-12-11  252  
1b1eeca7e4c19fa Ard Biesheuvel 2018-08-21  253  #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
3578ad11f3fba07 Sami Tolvanen  2020-12-11  254  #define ____define_initcall(fn, __stub, __name, __sec)		\
3578ad11f3fba07 Sami Tolvanen  2020-12-11  255  	__define_initcall_stub(__stub, fn)			\
a8cccdd954732a5 Sami Tolvanen  2020-12-11 @256  	asm(".section	\"" __sec "\", \"a\"		\n"	\
a8cccdd954732a5 Sami Tolvanen  2020-12-11  257  	    __stringify(__name) ":			\n"	\
3578ad11f3fba07 Sami Tolvanen  2020-12-11  258  	    ".long	" __stringify(__stub) " - .	\n"	\
1cb61759d407166 Marco Elver    2021-05-21  259  	    ".previous					\n");	\
1cb61759d407166 Marco Elver    2021-05-21  260  	static_assert(__same_type(initcall_t, &fn));
1b1eeca7e4c19fa Ard Biesheuvel 2018-08-21  261  #else
3578ad11f3fba07 Sami Tolvanen  2020-12-11  262  #define ____define_initcall(fn, __unused, __name, __sec)	\
a8cccdd954732a5 Sami Tolvanen  2020-12-11  263  	static initcall_t __name __used 			\
a8cccdd954732a5 Sami Tolvanen  2020-12-11  264  		__attribute__((__section__(__sec))) = fn;
1b1eeca7e4c19fa Ard Biesheuvel 2018-08-21  265  #endif
1b1eeca7e4c19fa Ard Biesheuvel 2018-08-21  266  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 19:01 ` [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS Ihor Solodrai
                     ` (2 preceding siblings ...)
  2025-10-30 10:24   ` kernel test robot
@ 2025-10-30 11:58   ` kernel test robot
  2025-10-30 13:54   ` kernel test robot
  4 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2025-10-30 11:58 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: llvm, oe-kbuild-all, dwarves, alan.maguire, acme, eddyz87, tj,
	kernel-team

Hi Ihor,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Ihor-Solodrai/bpf-Add-BTF_ID_LIST_END-and-BTF_ID_LIST_SIZE-macros/20251030-030608
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20251029190113.3323406-4-ihor.solodrai%40linux.dev
patch subject: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
config: s390-randconfig-001-20251030 (https://download.01.org/0day-ci/archive/20251030/202510301923.XmrKra1o-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251030/202510301923.XmrKra1o-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510301923.XmrKra1o-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/bpf/verifier.c:3273:1: error: invalid storage class specifier in function declarator
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
         | ^
   kernel/bpf/verifier.c:3273:12: error: parameter named 'magic_kfunc_by_impl' is missing
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
         |            ^
   kernel/bpf/verifier.c:3273:49: error: expected ';' at end of declaration
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
         |                                                 ^
         |                                                 ;
   kernel/bpf/verifier.c:3271:17: error: parameter 'magic_kfuncs' was not declared, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         |                 ^
    3272 | 
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
    3274 | {
   kernel/bpf/verifier.c:3271:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         | ^
         | int
   kernel/bpf/verifier.c:3277:18: error: call to undeclared function 'BTF_ID_LIST_SIZE'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3277 |         for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
         |                         ^
   kernel/bpf/verifier.c:3277:18: note: did you mean 'BTF_ID_LIST_END'?
   kernel/bpf/verifier.c:3271:1: note: 'BTF_ID_LIST_END' declared here
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         | ^
    3272 | 
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
    3274 | {
    3275 |         int i;
    3276 | 
    3277 |         for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
         |                         ~~~~~~~~~~~~~~~~
         |                         BTF_ID_LIST_END
   kernel/bpf/verifier.c:3278:19: error: subscripted value is not an array, pointer, or vector
    3278 |                 if (magic_kfuncs[i] == impl_func_id)
         |                     ~~~~~~~~~~~~^~
   kernel/bpf/verifier.c:3278:26: error: use of undeclared identifier 'impl_func_id'
    3278 |                 if (magic_kfuncs[i] == impl_func_id)
         |                                        ^
   kernel/bpf/verifier.c:3279:23: error: subscripted value is not an array, pointer, or vector
    3279 |                         return magic_kfuncs[i - 1];
         |                                ~~~~~~~~~~~~^~~~~~
>> kernel/bpf/verifier.c:3271:1: warning: no previous prototype for function 'BTF_ID_LIST_END' [-Wmissing-prototypes]
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         | ^
   kernel/bpf/verifier.c:3271:16: note: declare 'static' if the function is not intended to be used outside of this translation unit
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         |                ^
         |                static 
   kernel/bpf/verifier.c:3271:1: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype]
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         | ^
   kernel/bpf/verifier.c:3288:18: error: call to undeclared function 'BTF_ID_LIST_SIZE'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3288 |         for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
         |                         ^
   kernel/bpf/verifier.c:3409:17: error: call to undeclared function 'magic_kfunc_by_impl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3409 |                 tmp_func_id = magic_kfunc_by_impl(func_id);
         |                               ^
   kernel/bpf/verifier.c:13726:17: error: call to undeclared function 'magic_kfunc_by_impl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    13726 |                 tmp_func_id = magic_kfunc_by_impl(func_id);
          |                               ^
   1 warning and 13 errors generated.


vim +/BTF_ID_LIST_END +3271 kernel/bpf/verifier.c

  3265	
  3266	/*
  3267	 * magic_kfuncs is used as a list of (foo, foo_impl) pairs
  3268	 */
  3269	BTF_ID_LIST(magic_kfuncs)
  3270	BTF_ID_UNUSED
> 3271	BTF_ID_LIST_END(magic_kfuncs)
  3272	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 19:01 ` [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS Ihor Solodrai
                     ` (3 preceding siblings ...)
  2025-10-30 11:58   ` kernel test robot
@ 2025-10-30 13:54   ` kernel test robot
  4 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2025-10-30 13:54 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: llvm, oe-kbuild-all, dwarves, alan.maguire, acme, eddyz87, tj,
	kernel-team

Hi Ihor,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Ihor-Solodrai/bpf-Add-BTF_ID_LIST_END-and-BTF_ID_LIST_SIZE-macros/20251030-030608
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20251029190113.3323406-4-ihor.solodrai%40linux.dev
patch subject: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
config: s390-randconfig-001-20251030 (https://download.01.org/0day-ci/archive/20251030/202510302139.fAmMKkDb-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251030/202510302139.fAmMKkDb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510302139.fAmMKkDb-lkp@intel.com/

All errors (new ones prefixed by >>):

   kernel/bpf/verifier.c:3273:1: error: invalid storage class specifier in function declarator
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
         | ^
   kernel/bpf/verifier.c:3273:12: error: parameter named 'magic_kfunc_by_impl' is missing
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
         |            ^
   kernel/bpf/verifier.c:3273:49: error: expected ';' at end of declaration
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
         |                                                 ^
         |                                                 ;
   kernel/bpf/verifier.c:3271:17: error: parameter 'magic_kfuncs' was not declared, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         |                 ^
    3272 | 
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
    3274 | {
   kernel/bpf/verifier.c:3271:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         | ^
         | int
   kernel/bpf/verifier.c:3277:18: error: call to undeclared function 'BTF_ID_LIST_SIZE'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3277 |         for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
         |                         ^
   kernel/bpf/verifier.c:3277:18: note: did you mean 'BTF_ID_LIST_END'?
   kernel/bpf/verifier.c:3271:1: note: 'BTF_ID_LIST_END' declared here
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         | ^
    3272 | 
    3273 | static s32 magic_kfunc_by_impl(s32 impl_func_id)
    3274 | {
    3275 |         int i;
    3276 | 
    3277 |         for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
         |                         ~~~~~~~~~~~~~~~~
         |                         BTF_ID_LIST_END
   kernel/bpf/verifier.c:3278:19: error: subscripted value is not an array, pointer, or vector
    3278 |                 if (magic_kfuncs[i] == impl_func_id)
         |                     ~~~~~~~~~~~~^~
   kernel/bpf/verifier.c:3278:26: error: use of undeclared identifier 'impl_func_id'
    3278 |                 if (magic_kfuncs[i] == impl_func_id)
         |                                        ^
   kernel/bpf/verifier.c:3279:23: error: subscripted value is not an array, pointer, or vector
    3279 |                         return magic_kfuncs[i - 1];
         |                                ~~~~~~~~~~~~^~~~~~
   kernel/bpf/verifier.c:3271:1: warning: no previous prototype for function 'BTF_ID_LIST_END' [-Wmissing-prototypes]
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         | ^
   kernel/bpf/verifier.c:3271:16: note: declare 'static' if the function is not intended to be used outside of this translation unit
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         |                ^
         |                static 
>> kernel/bpf/verifier.c:3271:1: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype]
    3271 | BTF_ID_LIST_END(magic_kfuncs)
         | ^
   kernel/bpf/verifier.c:3288:18: error: call to undeclared function 'BTF_ID_LIST_SIZE'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3288 |         for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
         |                         ^
>> kernel/bpf/verifier.c:3409:17: error: call to undeclared function 'magic_kfunc_by_impl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3409 |                 tmp_func_id = magic_kfunc_by_impl(func_id);
         |                               ^
   kernel/bpf/verifier.c:13726:17: error: call to undeclared function 'magic_kfunc_by_impl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    13726 |                 tmp_func_id = magic_kfunc_by_impl(func_id);
          |                               ^
   1 warning and 13 errors generated.


vim +3271 kernel/bpf/verifier.c

  3265	
  3266	/*
  3267	 * magic_kfuncs is used as a list of (foo, foo_impl) pairs
  3268	 */
  3269	BTF_ID_LIST(magic_kfuncs)
  3270	BTF_ID_UNUSED
> 3271	BTF_ID_LIST_END(magic_kfuncs)
  3272	
  3273	static s32 magic_kfunc_by_impl(s32 impl_func_id)
  3274	{
  3275		int i;
  3276	
  3277		for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
  3278			if (magic_kfuncs[i] == impl_func_id)
  3279				return magic_kfuncs[i - 1];
  3280		}
  3281		return -ENOENT;
  3282	}
  3283	
  3284	static s32 impl_by_magic_kfunc(s32 func_id)
  3285	{
  3286		int i;
  3287	
  3288		for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
  3289			if (magic_kfuncs[i] == func_id)
  3290				return magic_kfuncs[i + 1];
  3291		}
  3292		return -ENOENT;
  3293	}
  3294	
  3295	static const struct btf_type *find_magic_kfunc_proto(struct btf *desc_btf, s32 func_id)
  3296	{
  3297		const struct btf_type *impl_func, *func_proto;
  3298		u32 impl_func_id;
  3299	
  3300		impl_func_id = impl_by_magic_kfunc(func_id);
  3301		if (impl_func_id < 0)
  3302			return NULL;
  3303	
  3304		impl_func = btf_type_by_id(desc_btf, impl_func_id);
  3305		if (!impl_func || !btf_type_is_func(impl_func))
  3306			return NULL;
  3307	
  3308		func_proto = btf_type_by_id(desc_btf, impl_func->type);
  3309		if (!func_proto || !btf_type_is_func_proto(func_proto))
  3310			return NULL;
  3311	
  3312		return func_proto;
  3313	}
  3314	
  3315	static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
  3316	{
  3317		const struct btf_type *func, *func_proto, *tmp_func;
  3318		struct bpf_kfunc_btf_tab *btf_tab;
  3319		const char *func_name, *tmp_name;
  3320		struct btf_func_model func_model;
  3321		struct bpf_kfunc_desc_tab *tab;
  3322		struct bpf_prog_aux *prog_aux;
  3323		struct bpf_kfunc_desc *desc;
  3324		struct btf *desc_btf;
  3325		unsigned long addr;
  3326		u32 *kfunc_flags;
  3327		s32 tmp_func_id;
  3328		int err;
  3329	
  3330		prog_aux = env->prog->aux;
  3331		tab = prog_aux->kfunc_tab;
  3332		btf_tab = prog_aux->kfunc_btf_tab;
  3333		if (!tab) {
  3334			if (!btf_vmlinux) {
  3335				verbose(env, "calling kernel function is not supported without CONFIG_DEBUG_INFO_BTF\n");
  3336				return -ENOTSUPP;
  3337			}
  3338	
  3339			if (!env->prog->jit_requested) {
  3340				verbose(env, "JIT is required for calling kernel function\n");
  3341				return -ENOTSUPP;
  3342			}
  3343	
  3344			if (!bpf_jit_supports_kfunc_call()) {
  3345				verbose(env, "JIT does not support calling kernel function\n");
  3346				return -ENOTSUPP;
  3347			}
  3348	
  3349			if (!env->prog->gpl_compatible) {
  3350				verbose(env, "cannot call kernel function from non-GPL compatible program\n");
  3351				return -EINVAL;
  3352			}
  3353	
  3354			tab = kzalloc(sizeof(*tab), GFP_KERNEL_ACCOUNT);
  3355			if (!tab)
  3356				return -ENOMEM;
  3357			prog_aux->kfunc_tab = tab;
  3358		}
  3359	
  3360		/* func_id == 0 is always invalid, but instead of returning an error, be
  3361		 * conservative and wait until the code elimination pass before returning
  3362		 * error, so that invalid calls that get pruned out can be in BPF programs
  3363		 * loaded from userspace.  It is also required that offset be untouched
  3364		 * for such calls.
  3365		 */
  3366		if (!func_id && !offset)
  3367			return 0;
  3368	
  3369		if (!btf_tab && offset) {
  3370			btf_tab = kzalloc(sizeof(*btf_tab), GFP_KERNEL_ACCOUNT);
  3371			if (!btf_tab)
  3372				return -ENOMEM;
  3373			prog_aux->kfunc_btf_tab = btf_tab;
  3374		}
  3375	
  3376		desc_btf = find_kfunc_desc_btf(env, offset);
  3377		if (IS_ERR(desc_btf)) {
  3378			verbose(env, "failed to find BTF for kernel function\n");
  3379			return PTR_ERR(desc_btf);
  3380		}
  3381	
  3382		if (find_kfunc_desc(env->prog, func_id, offset))
  3383			return 0;
  3384	
  3385		if (tab->nr_descs == MAX_KFUNC_DESCS) {
  3386			verbose(env, "too many different kernel function calls\n");
  3387			return -E2BIG;
  3388		}
  3389	
  3390		func = btf_type_by_id(desc_btf, func_id);
  3391		if (!func || !btf_type_is_func(func)) {
  3392			verbose(env, "kernel btf_id %u is not a function\n",
  3393				func_id);
  3394			return -EINVAL;
  3395		}
  3396		func_proto = btf_type_by_id(desc_btf, func->type);
  3397		if (!func_proto || !btf_type_is_func_proto(func_proto)) {
  3398			verbose(env, "kernel function btf_id %u does not have a valid func_proto\n",
  3399				func_id);
  3400			return -EINVAL;
  3401		}
  3402	
  3403		kfunc_flags = btf_kfunc_flags(desc_btf, func_id, env->prog);
  3404		func_name = btf_name_by_offset(desc_btf, func->name_off);
  3405		addr = kallsyms_lookup_name(func_name);
  3406	
  3407		/* This may be an _impl kfunc with KF_MAGIC_ARGS counterpart */
  3408		if (unlikely(!addr && !kfunc_flags)) {
> 3409			tmp_func_id = magic_kfunc_by_impl(func_id);
  3410			if (tmp_func_id < 0)
  3411				return -EACCES;
  3412			tmp_func = btf_type_by_id(desc_btf, tmp_func_id);
  3413			if (!tmp_func || !btf_type_is_func(tmp_func))
  3414				return -EACCES;
  3415			tmp_name = btf_name_by_offset(desc_btf, tmp_func->name_off);
  3416			addr = kallsyms_lookup_name(tmp_name);
  3417		}
  3418	
  3419		/*
  3420		 * Note that kfunc_flags may be NULL at this point, which means that we couldn't find
  3421		 * func_id in any relevant kfunc_id_set. This most likely indicates an invalid kfunc call.
  3422		 * However we don't want to fail the verification here, because invalid calls may be
  3423		 * eliminated as dead code later.
  3424		 */
  3425		if (unlikely(kfunc_flags && KF_MAGIC_ARGS & *kfunc_flags)) {
  3426			func_proto = find_magic_kfunc_proto(desc_btf, func_id);
  3427			if (!func_proto) {
  3428				verbose(env, "cannot find _impl proto for kernel function %s\n",
  3429				func_name);
  3430				return -EINVAL;
  3431			}
  3432		}
  3433	
  3434		if (!addr) {
  3435			verbose(env, "cannot find address for kernel function %s\n",
  3436				func_name);
  3437			return -EINVAL;
  3438		}
  3439	
  3440		if (bpf_dev_bound_kfunc_id(func_id)) {
  3441			err = bpf_dev_bound_kfunc_check(&env->log, prog_aux);
  3442			if (err)
  3443				return err;
  3444		}
  3445	
  3446		err = btf_distill_func_proto(&env->log, desc_btf,
  3447					     func_proto, func_name,
  3448					     &func_model);
  3449		if (err)
  3450			return err;
  3451	
  3452		desc = &tab->descs[tab->nr_descs++];
  3453		desc->func_id = func_id;
  3454		desc->offset = offset;
  3455		desc->addr = addr;
  3456		desc->func_model = func_model;
  3457		sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
  3458		     kfunc_desc_cmp_by_id_off, NULL);
  3459		return 0;
  3460	}
  3461	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-29 23:54   ` Eduard Zingerman
  2025-10-30  0:03     ` Alexei Starovoitov
@ 2025-10-30 16:31     ` Ihor Solodrai
  2025-10-30 17:26       ` Eduard Zingerman
  1 sibling, 1 reply; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-30 16:31 UTC (permalink / raw)
  To: Eduard Zingerman, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

Hi Eduard, thank you for a quick review.

On 10/29/25 4:54 PM, Eduard Zingerman wrote:
> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
>> A kernel function bpf_foo with KF_MAGIC_ARGS flag is expected to have
>                                  ^^^^^^^^^^^^^
> 		I don't like this name very much.
> 		It bears very little context.
> 		Imo, KF_IMPLICIT_ARGS fits the use case much better.

I know, naming is hard...

The issue is that it's not only the flag, across the code we need
descriptive names for every "magic" thing:
  * a flagged function
    * how do we call it? kfunc_with_impl_args?
  * a function that exists only in BTF (_impl)
    * it's not an "implicit" function
    * it's not exactly an "implementation" function
    * "fake" is even worse than "magic" IMO, because it's not fake,
      but you could argue it's magical :D
    * btf_only_kfunc?
  * describing arguments is simpler: "implicit" seems ok, although as
    Alexei pointed out in previous iteration they are very much
    explicit in the kernel [1]

For me, "(BPF) interface" and "(kernel) implementation" pair of terms
makes sense, but then I think it would be logical to have both
declarations in the kernel.

The advantage of "magic" in this context is that it doesn't have
loaded meaning. But I agree this is a stretch, so can't insist.

[1] https://lore.kernel.org/bpf/CAADnVQLvuubey0A0Fk=bzN-=JG2UUQHRqBijZpuvqMQ+xy4W4g@mail.gmail.com/


> 
>> two types in BTF:
>>   * `bpf_foo` with a function prototype that omits __magic arguments
>>   * `bpf_foo_impl` with a function prototype that matches kernel
>>      declaration, but doesn't have a ksym associated with its name
> 
> Could you please start with an example here?
> Stating how `bpf_foo` needs to be declared in kernel, and what are the
> options to invoke it from bpf.  Then proceed with BTF details, etc.

Ok. I think I can reshuffle explanations between the cover letter and
commit message, it's a bit redundant already.

> 
>> In order to support magic kfuncs the verifier has to know how to
>> resolve calls both of `bpf_foo` and `bpf_foo_impl` to the correct BTF
>> function prototype and address.
>>
>> In add_kfunc_call() kfunc flags are inspected to detect a magic kfunc
>> or its _impl, and then the address and func_proto are adjusted for the
>> kfunc descriptor.
>>
>> In fetch_kfunc_meta() similar logic is used to fixup the contents of
>> struct bpf_kfunc_call_arg_meta.
>>
>> In check_kfunc_call() reset the subreg_def of registers holding magic
>> arguments to correctly track zero extensions.
>>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
>> ---
>>  include/linux/btf.h   |   1 +
>>  kernel/bpf/verifier.c | 123 ++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 120 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/btf.h b/include/linux/btf.h
>> index 9c64bc5e5789..3fe20514692f 100644
>> --- a/include/linux/btf.h
>> +++ b/include/linux/btf.h
>> @@ -79,6 +79,7 @@
>>  #define KF_ARENA_RET    (1 << 13) /* kfunc returns an arena pointer */
>>  #define KF_ARENA_ARG1   (1 << 14) /* kfunc takes an arena pointer as its first argument */
>>  #define KF_ARENA_ARG2   (1 << 15) /* kfunc takes an arena pointer as its second argument */
>> +#define KF_MAGIC_ARGS   (1 << 16) /* kfunc signature is different from its BPF signature */
>>  
>>  /*
>>   * Tag marking a kernel function as a kfunc. This is meant to minimize the
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index cb1b483be0fa..fcf0872b9e3d 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -3263,17 +3263,68 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
>>  	return btf_vmlinux ?: ERR_PTR(-ENOENT);
>>  }
>>  
>> +/*
>> + * magic_kfuncs is used as a list of (foo, foo_impl) pairs
>> + */
>> +BTF_ID_LIST(magic_kfuncs)
>> +BTF_ID_UNUSED
>> +BTF_ID_LIST_END(magic_kfuncs)
>> +
>> +static s32 magic_kfunc_by_impl(s32 impl_func_id)
>> +{
>> +	int i;
>> +
>> +	for (i = 1; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
>> +		if (magic_kfuncs[i] == impl_func_id)
>                                        ^^^^^
> Nit: similarly, I'd rename this to something like "implicit_func_id"
>      or "fake_func_id. "impl" is confusing because this id has nothing
>      to do with implementation.
> 
>> +			return magic_kfuncs[i - 1];
>> +	}
>> +	return -ENOENT;
>> +}
>> +
>> +static s32 impl_by_magic_kfunc(s32 func_id)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < BTF_ID_LIST_SIZE(magic_kfuncs); i += 2) {
>> +		if (magic_kfuncs[i] == func_id)
>> +			return magic_kfuncs[i + 1];
>> +	}
>> +	return -ENOENT;
>> +}
>> +
>> +static const struct btf_type *find_magic_kfunc_proto(struct btf *desc_btf, s32 func_id)
>> +{
>> +	const struct btf_type *impl_func, *func_proto;
>> +	u32 impl_func_id;
>> +
>> +	impl_func_id = impl_by_magic_kfunc(func_id);
>> +	if (impl_func_id < 0)
>> +		return NULL;
>> +
>> +	impl_func = btf_type_by_id(desc_btf, impl_func_id);
>> +	if (!impl_func || !btf_type_is_func(impl_func))
>> +		return NULL;
>> +
>> +	func_proto = btf_type_by_id(desc_btf, impl_func->type);
>> +	if (!func_proto || !btf_type_is_func_proto(func_proto))
>> +		return NULL;
>> +
>> +	return func_proto;
>> +}
>> +
>>  static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
>>  {
>> -	const struct btf_type *func, *func_proto;
>> +	const struct btf_type *func, *func_proto, *tmp_func;
>>  	struct bpf_kfunc_btf_tab *btf_tab;
>> +	const char *func_name, *tmp_name;
>>  	struct btf_func_model func_model;
>>  	struct bpf_kfunc_desc_tab *tab;
>>  	struct bpf_prog_aux *prog_aux;
>>  	struct bpf_kfunc_desc *desc;
>> -	const char *func_name;
>>  	struct btf *desc_btf;
>>  	unsigned long addr;
>> +	u32 *kfunc_flags;
>> +	s32 tmp_func_id;
>>  	int err;
>>  
>>  	prog_aux = env->prog->aux;
>> @@ -3349,8 +3400,37 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
>>  		return -EINVAL;
>>  	}
>>  
>> +	kfunc_flags = btf_kfunc_flags(desc_btf, func_id, env->prog);
>>  	func_name = btf_name_by_offset(desc_btf, func->name_off);
>>  	addr = kallsyms_lookup_name(func_name);
>> +
>> +	/* This may be an _impl kfunc with KF_MAGIC_ARGS counterpart */
>> +	if (unlikely(!addr && !kfunc_flags)) {
>> +		tmp_func_id = magic_kfunc_by_impl(func_id);
> 
> I think there is no need to hide magic_kfunc_by_impl() call behind the
> above condition. It can be moved before kfunc_flags assignment.
> Then it wont be necessary to textually repeat btf_name_by_offset() and
> kallsyms_lookup_name() calls.

Not sure I follow...

Yes, !addr is enough to detect potential _impl function, but there is
no way around name lookup in BTF and then another address lookup.

The _impl function doesn't have an address, so after failed
  kallsyms_lookup_name("kfunc_impl");
we must do
  kallsyms_lookup_name("kfunc");
to find the correct address.

Or do you suggest doing something like:

  tmp_func_id = magic_kfunc_by_impl(func_id);
  if (tmp_func_id > 0)
      func_id = tmp_func_id;

at the beginning of add_kfunc_call()?


> 
>> +		if (tmp_func_id < 0)
>> +			return -EACCES;
> 
> Nit: this skips proper error reporting: "cannot find address for kernel function %s\n".
> 
>> +		tmp_func = btf_type_by_id(desc_btf, tmp_func_id);
>> +		if (!tmp_func || !btf_type_is_func(tmp_func))
> 
> Nit: this condition indicates a verifier bug, should it be reported as such?
> 
>> +			return -EACCES;
>> +		tmp_name = btf_name_by_offset(desc_btf, tmp_func->name_off);
>> +		addr = kallsyms_lookup_name(tmp_name);
>> +	}
>> +
>> +	/*
>> +	 * Note that kfunc_flags may be NULL at this point, which means that we couldn't find
>> +	 * func_id in any relevant kfunc_id_set. This most likely indicates an invalid kfunc call.
>> +	 * However we don't want to fail the verification here, because invalid calls may be
>> +	 * eliminated as dead code later.
>> +	 */
>> +	if (unlikely(kfunc_flags && KF_MAGIC_ARGS & *kfunc_flags)) {
>> +		func_proto = find_magic_kfunc_proto(desc_btf, func_id);
>> +		if (!func_proto) {
>> +			verbose(env, "cannot find _impl proto for kernel function %s\n",
>> +			func_name);
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>>  	if (!addr) {
>>  		verbose(env, "cannot find address for kernel function %s\n",
>>  			func_name);
>> @@ -12051,6 +12131,11 @@ static bool is_kfunc_arg_irq_flag(const struct btf *btf, const struct btf_param
>>  	return btf_param_match_suffix(btf, arg, "__irq_flag");
>>  }
>>  
>> +static bool is_kfunc_arg_magic(const struct btf *btf, const struct btf_param *arg)
>> +{
>> +	return btf_param_match_suffix(btf, arg, "__magic");
>> +}
>> +
>>  static bool is_kfunc_arg_prog(const struct btf *btf, const struct btf_param *arg)
>>  {
>>  	return btf_param_match_suffix(btf, arg, "__prog");
>> @@ -13613,6 +13698,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
>>  	u32 func_id, *kfunc_flags;
>>  	const char *func_name;
>>  	struct btf *desc_btf;
>> +	s32 tmp_func_id;
>>  
>>  	if (kfunc_name)
>>  		*kfunc_name = NULL;
>> @@ -13632,10 +13718,28 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
>>  	func_proto = btf_type_by_id(desc_btf, func->type);
>>  
>>  	kfunc_flags = btf_kfunc_flags_if_allowed(desc_btf, func_id, env->prog);
>> -	if (!kfunc_flags) {
>> -		return -EACCES;
>> +	if (unlikely(!kfunc_flags)) {
> 
> What if we patch insn->imm to use the "fake" function id in add_kfunc_call()?
> Then modifications to fetch_kfunc_meta() wont be necessary.


I considered this. I wasn't sure it's safe to patch insn->imm at this
stage of verification. Also I thought it may be harder to debug the
verifier if we do btf id replacement in the calls pre-verification
(because we lose the original btf id).

Maybe I was too causious.

Alexei, Andrii, what do you think?


> 
>> +		/*
>> +		 * An _impl kfunc with KF_MAGIC_ARGS counterpart
>> +		 * does not have its own kfunc flags.
>> +		 */
>> +		tmp_func_id = magic_kfunc_by_impl(func_id);
>> +		if (tmp_func_id < 0)
>> +			return -EACCES;
>> +		kfunc_flags = btf_kfunc_flags_if_allowed(desc_btf, tmp_func_id, env->prog);
>> +		if (!kfunc_flags)
>> +			return -EACCES;
>> +	} else if (unlikely(KF_MAGIC_ARGS & *kfunc_flags)) {
>> +		/*
>> +		 * An actual func_proto of a kfunc with KF_MAGIC_ARGS flag
>> +		 * can be found through the corresponding _impl kfunc.
>> +		 */
>> +		func_proto = find_magic_kfunc_proto(desc_btf, func_id);
>>  	}
>>  
>> +	if (!func_proto)
>> +		return -EACCES;
>> +
>>  	memset(meta, 0, sizeof(*meta));
>>  	meta->btf = desc_btf;
>>  	meta->func_id = func_id;
>> @@ -14189,6 +14293,17 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>  	for (i = 0; i < nargs; i++) {
>>  		u32 regno = i + 1;
>>  
>> +		/*
>> +		 * Magic arguments are set after main verification pass.
>> +		 * For correct tracking of zero-extensions we have to reset subreg_def for such
>> +		 * args. Otherwise mark_btf_func_reg_size() will be inspecting subreg_def of regs
>> +		 * from an earlier (irrelevant) point in the program, which may lead to an error
>> +		 * in opt_subreg_zext_lo32_rnd_hi32().
>> +		 */
>> +		if (unlikely(KF_MAGIC_ARGS & meta.kfunc_flags
>> +				&& is_kfunc_arg_magic(desc_btf, &args[i])))
>> +			regs[regno].subreg_def = DEF_NOT_SUBREG;
>> +
>>  		t = btf_type_skip_modifiers(desc_btf, args[i].type, NULL);
>>  		if (btf_type_is_ptr(t))
>>  			mark_btf_func_reg_size(env, regno, sizeof(void *));


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS
  2025-10-30 16:31     ` Ihor Solodrai
@ 2025-10-30 17:26       ` Eduard Zingerman
  0 siblings, 0 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-30 17:26 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Thu, 2025-10-30 at 09:31 -0700, Ihor Solodrai wrote:
> Hi Eduard, thank you for a quick review.
> 
> On 10/29/25 4:54 PM, Eduard Zingerman wrote:
> > On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> > > A kernel function bpf_foo with KF_MAGIC_ARGS flag is expected to have
> >                                  ^^^^^^^^^^^^^
> > 		I don't like this name very much.
> > 		It bears very little context.
> > 		Imo, KF_IMPLICIT_ARGS fits the use case much better.
> 
> I know, naming is hard...
> 
> The issue is that it's not only the flag, across the code we need
> descriptive names for every "magic" thing:
>   * a flagged function
>     * how do we call it? kfunc_with_impl_args?
>   * a function that exists only in BTF (_impl)
>     * it's not an "implicit" function
>     * it's not exactly an "implementation" function
>     * "fake" is even worse than "magic" IMO, because it's not fake,
>       but you could argue it's magical :D
>     * btf_only_kfunc?
>   * describing arguments is simpler: "implicit" seems ok, although as
>     Alexei pointed out in previous iteration they are very much
>     explicit in the kernel [1]
> 
> For me, "(BPF) interface" and "(kernel) implementation" pair of terms
> makes sense, but then I think it would be logical to have both
> declarations in the kernel.
> 
> The advantage of "magic" in this context is that it doesn't have
> loaded meaning. But I agree this is a stretch, so can't insist.
> 
> [1] https://lore.kernel.org/bpf/CAADnVQLvuubey0A0Fk=bzN-=JG2UUQHRqBijZpuvqMQ+xy4W4g@mail.gmail.com/

- KF_IMPLICIT_ARGS
- explicit_args_id -- for prototype with full set of args
- implicit_args_id -- for prototype with missing args

[...]

> > > @@ -3349,8 +3400,37 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > +	kfunc_flags = btf_kfunc_flags(desc_btf, func_id, env->prog);
> > >  	func_name = btf_name_by_offset(desc_btf, func->name_off);
> > >  	addr = kallsyms_lookup_name(func_name);
> > > +
> > > +	/* This may be an _impl kfunc with KF_MAGIC_ARGS counterpart */
> > > +	if (unlikely(!addr && !kfunc_flags)) {
> > > +		tmp_func_id = magic_kfunc_by_impl(func_id);
> > 
> > I think there is no need to hide magic_kfunc_by_impl() call behind the
> > above condition. It can be moved before kfunc_flags assignment.
> > Then it wont be necessary to textually repeat btf_name_by_offset() and
> > kallsyms_lookup_name() calls.
> 
> Not sure I follow...
> 
> Yes, !addr is enough to detect potential _impl function, but there is
> no way around name lookup in BTF and then another address lookup.
> 
> The _impl function doesn't have an address, so after failed
>   kallsyms_lookup_name("kfunc_impl");
> we must do
>   kallsyms_lookup_name("kfunc");
> to find the correct address.
> 
> Or do you suggest doing something like:
> 
>   tmp_func_id = magic_kfunc_by_impl(func_id);
>   if (tmp_func_id > 0)
>       func_id = tmp_func_id;
> 
> at the beginning of add_kfunc_call()?

This, just check for magic_kfunc_by_impl() and replace func_id.

[...]

> > > @@ -13632,10 +13718,28 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
> > >  	func_proto = btf_type_by_id(desc_btf, func->type);
> > >  
> > >  	kfunc_flags = btf_kfunc_flags_if_allowed(desc_btf, func_id, env->prog);
> > > -	if (!kfunc_flags) {
> > > -		return -EACCES;
> > > +	if (unlikely(!kfunc_flags)) {
> > 
> > What if we patch insn->imm to use the "fake" function id in add_kfunc_call()?
> > Then modifications to fetch_kfunc_meta() wont be necessary.
> 
> 
> I considered this. I wasn't sure it's safe to patch insn->imm at this
> stage of verification. Also I thought it may be harder to debug the
> verifier if we do btf id replacement in the calls pre-verification
> (because we lose the original btf id).

See no issues with that.

> Maybe I was too causious.
> 
> Alexei, Andrii, what do you think?

[...]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30  6:11   ` Eduard Zingerman
@ 2025-10-30 18:14     ` Eduard Zingerman
  2025-10-30 18:24       ` Ihor Solodrai
  2025-10-30 18:26       ` Alan Maguire
  0 siblings, 2 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-30 18:14 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Wed, 2025-10-29 at 23:11 -0700, Eduard Zingerman wrote:
> On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
> > On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> > 
> > Do we break compatibility with old pahole versions after this
> > patch-set? Old paholes won't synthesize the _impl kfuncs, so:
> > - binary compatibility between new-kernel/old-pahole + old-bpf
> >   will be broken, as there would be no _impl kfuncs;
> > - new-kernel/old-pahole + new-bpf won't work either, as kernel will
> > be
> >   unable to find non-_impl function names for existing kfuncs.
> > 
> > [...]
> 
> Point being, if we are going to break backwards compatibility the
> following things need an update:
> - Documentation/process/changes.rst
>   minimal pahole version has to be bumped
> - scripts/Makefile.btf
>   All the different flags and options for different pahole
>   versions can be dropped.
> 
> ---
> 
> On the other hand, I'm not sure this useful but relatively obscure
> feature grants such a compatibility break. Some time ago Ihor
> advocated for just having two functions in the kernel, so that BTF
> will be generated for both. And I think that someone suggested putting
> the fake function to a discard-able section.
> This way the whole thing can be done in kernel only.
> E.g. it will look like so:
> 
>   __bpf_kfunc void btf_foo_impl(struct bpf_prog_aux p__implicit)
>   { /* real impl here */ }
> 
>   __bpf_kfunc_proto void btf_foo(void) {}
> 
> Assuming that __bpf_kfunc_proto hides all the necessary attributes.
> Not much boilerplate, and a tad easier to understand where second
> prototype comes from, no need to read pahole.

Scheme discussed off-list for new functions with __implicit args:
- kernel source code:

    __bpf_kfunc void foo(struct bpf_prog_aux p__implicit)
	BTF_ID_FLAGS(foo, KF_IMPLICIT_ARGS)

- pahole:
  - renames foo to foo_impl
  - adds bpf-side definition for 'foo' w/o implicit args
  vmlinux btf:

    __bpf_kfunc void foo_impl(struct bpf_prog_aux p__implicit);
    void foo(void);

- resolve_btfids puts the 'foo' (the one w/o implicit args) id to all
  id lists (no changes needed for this, follows from pahole changes);
- verifier.c:add_kfunc_call()
  - Sees the id of 'foo' and kfunc flags with KF_IMPLICIT_ARGS
  - Replaces the id with id of 'foo_impl'.

This will break the following scenario:
- new kfunc is added with __implicit arg
- kernel is built with old pahole
- vmlinux.h is generated for such kernel
- bpf program is compiled against such vmlinux.h
- attempt to run such program on a new kernel compiled with new pahole
  will fail.

Andrei and Alexei deemed this acceptable.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30 18:14     ` Eduard Zingerman
@ 2025-10-30 18:24       ` Ihor Solodrai
  2025-10-30 18:37         ` Eduard Zingerman
  2025-10-30 18:26       ` Alan Maguire
  1 sibling, 1 reply; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-30 18:24 UTC (permalink / raw)
  To: Eduard Zingerman, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On 10/30/25 11:14 AM, Eduard Zingerman wrote:
> On Wed, 2025-10-29 at 23:11 -0700, Eduard Zingerman wrote:
>> On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
>>> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
>>>
>>> Do we break compatibility with old pahole versions after this
>>> patch-set? Old paholes won't synthesize the _impl kfuncs, so:
>>> - binary compatibility between new-kernel/old-pahole + old-bpf
>>>   will be broken, as there would be no _impl kfuncs;
>>> - new-kernel/old-pahole + new-bpf won't work either, as kernel will
>>> be
>>>   unable to find non-_impl function names for existing kfuncs.
>>>
>>> [...]
>>
>> Point being, if we are going to break backwards compatibility the
>> following things need an update:
>> - Documentation/process/changes.rst
>>   minimal pahole version has to be bumped
>> - scripts/Makefile.btf
>>   All the different flags and options for different pahole
>>   versions can be dropped.
>>
>> ---
>>
>> On the other hand, I'm not sure this useful but relatively obscure
>> feature grants such a compatibility break. Some time ago Ihor
>> advocated for just having two functions in the kernel, so that BTF
>> will be generated for both. And I think that someone suggested putting
>> the fake function to a discard-able section.
>> This way the whole thing can be done in kernel only.
>> E.g. it will look like so:
>>
>>   __bpf_kfunc void btf_foo_impl(struct bpf_prog_aux p__implicit)
>>   { /* real impl here */ }
>>
>>   __bpf_kfunc_proto void btf_foo(void) {}
>>
>> Assuming that __bpf_kfunc_proto hides all the necessary attributes.
>> Not much boilerplate, and a tad easier to understand where second
>> prototype comes from, no need to read pahole.
> 
> Scheme discussed off-list for new functions with __implicit args:
> - kernel source code:
> 
>     __bpf_kfunc void foo(struct bpf_prog_aux p__implicit)
> 	BTF_ID_FLAGS(foo, KF_IMPLICIT_ARGS)
> 
> - pahole:
>   - renames foo to foo_impl
>   - adds bpf-side definition for 'foo' w/o implicit args
>   vmlinux btf:
> 
>     __bpf_kfunc void foo_impl(struct bpf_prog_aux p__implicit);
>     void foo(void);

I believe it's the other way around:
     void foo_impl(struct bpf_prog_aux p__implicit);
     __bpf_kfunc void foo(void);

foo() is callable from BPF, but foo_impl() is not.
But we still want foo_impl() in BTF so that the verifier can find the 
correct prototype.

Andrii, please confirm.

> 
> - resolve_btfids puts the 'foo' (the one w/o implicit args) id to all
>   id lists (no changes needed for this, follows from pahole changes);
> - verifier.c:add_kfunc_call()
>   - Sees the id of 'foo' and kfunc flags with KF_IMPLICIT_ARGS
>   - Replaces the id with id of 'foo_impl'.
> 
> This will break the following scenario:
> - new kfunc is added with __implicit arg
> - kernel is built with old pahole
> - vmlinux.h is generated for such kernel
> - bpf program is compiled against such vmlinux.h
> - attempt to run such program on a new kernel compiled with new pahole
>   will fail.

I think you meant "new kernel compiled with old pahole".

> 
> Andrei and Alexei deemed this acceptable.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30 18:14     ` Eduard Zingerman
  2025-10-30 18:24       ` Ihor Solodrai
@ 2025-10-30 18:26       ` Alan Maguire
  2025-10-30 18:42         ` Eduard Zingerman
  2025-10-30 18:46         ` Ihor Solodrai
  1 sibling, 2 replies; 36+ messages in thread
From: Alan Maguire @ 2025-10-30 18:26 UTC (permalink / raw)
  To: Eduard Zingerman, Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, acme, tj, kernel-team

On 30/10/2025 18:14, Eduard Zingerman wrote:
> On Wed, 2025-10-29 at 23:11 -0700, Eduard Zingerman wrote:
>> On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
>>> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
>>>
>>> Do we break compatibility with old pahole versions after this
>>> patch-set? Old paholes won't synthesize the _impl kfuncs, so:
>>> - binary compatibility between new-kernel/old-pahole + old-bpf
>>>   will be broken, as there would be no _impl kfuncs;
>>> - new-kernel/old-pahole + new-bpf won't work either, as kernel will
>>> be
>>>   unable to find non-_impl function names for existing kfuncs.
>>>
>>> [...]
>>
>> Point being, if we are going to break backwards compatibility the
>> following things need an update:
>> - Documentation/process/changes.rst
>>   minimal pahole version has to be bumped
>> - scripts/Makefile.btf
>>   All the different flags and options for different pahole
>>   versions can be dropped.
>>
>> ---
>>
>> On the other hand, I'm not sure this useful but relatively obscure
>> feature grants such a compatibility break. Some time ago Ihor
>> advocated for just having two functions in the kernel, so that BTF
>> will be generated for both. And I think that someone suggested putting
>> the fake function to a discard-able section.
>> This way the whole thing can be done in kernel only.
>> E.g. it will look like so:
>>
>>   __bpf_kfunc void btf_foo_impl(struct bpf_prog_aux p__implicit)
>>   { /* real impl here */ }
>>
>>   __bpf_kfunc_proto void btf_foo(void) {}
>>
>> Assuming that __bpf_kfunc_proto hides all the necessary attributes.
>> Not much boilerplate, and a tad easier to understand where second
>> prototype comes from, no need to read pahole.
> 
> Scheme discussed off-list for new functions with __implicit args:
> - kernel source code:
> 
>     __bpf_kfunc void foo(struct bpf_prog_aux p__implicit)
> 	BTF_ID_FLAGS(foo, KF_IMPLICIT_ARGS)
> 
> - pahole:
>   - renames foo to foo_impl
>   - adds bpf-side definition for 'foo' w/o implicit args
>   vmlinux btf:
> 
>     __bpf_kfunc void foo_impl(struct bpf_prog_aux p__implicit);
>     void foo(void);
> 
> - resolve_btfids puts the 'foo' (the one w/o implicit args) id to all
>   id lists (no changes needed for this, follows from pahole changes);
> - verifier.c:add_kfunc_call()
>   - Sees the id of 'foo' and kfunc flags with KF_IMPLICIT_ARGS
>   - Replaces the id with id of 'foo_impl'.
> 
> This will break the following scenario:
> - new kfunc is added with __implicit arg
> - kernel is built with old pahole
> - vmlinux.h is generated for such kernel
> - bpf program is compiled against such vmlinux.h
> - attempt to run such program on a new kernel compiled with new pahole
>   will fail.
> 
> Andrei and Alexei deemed this acceptable.

Okay so bear with me as this is probably a massive over-simplification.
It seems like what we want here is a way to establish a relationship
between the BTF associated with the _impl function and the kfunc-visible
form (without the implicit arguments), right? Once we have that
relationship, it's sort of implicit which are the implicit arguments;
they're the ones the _impl variant has and the non-impl variant doesn't
have. So to me - and again I'm probably missing a lot - the key thing is
to establish that relationship between kfunc and kfunc_impl. Couldn't we
leverage the kernel build machinery around resolve_btf_ids to construct
these pairwise mappings of BTF ids? That way we keep pahole out of the
loop (aside from generating BTF for both variants as usual) and
compatibility issues aren't there as much because resolve_btfids travels
with the kernel, no changes needed for pahole.

I'm guessing the above is missing something though?

Thanks!

Alan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30 18:24       ` Ihor Solodrai
@ 2025-10-30 18:37         ` Eduard Zingerman
  0 siblings, 0 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-30 18:37 UTC (permalink / raw)
  To: Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, alan.maguire, acme, tj, kernel-team

On Thu, 2025-10-30 at 11:24 -0700, Ihor Solodrai wrote:
> On 10/30/25 11:14 AM, Eduard Zingerman wrote:
> > On Wed, 2025-10-29 at 23:11 -0700, Eduard Zingerman wrote:
> > > On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
> > > > On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> > > > 
> > > > Do we break compatibility with old pahole versions after this
> > > > patch-set? Old paholes won't synthesize the _impl kfuncs, so:
> > > > - binary compatibility between new-kernel/old-pahole + old-bpf
> > > >   will be broken, as there would be no _impl kfuncs;
> > > > - new-kernel/old-pahole + new-bpf won't work either, as kernel will
> > > > be
> > > >   unable to find non-_impl function names for existing kfuncs.
> > > > 
> > > > [...]
> > > 
> > > Point being, if we are going to break backwards compatibility the
> > > following things need an update:
> > > - Documentation/process/changes.rst
> > >   minimal pahole version has to be bumped
> > > - scripts/Makefile.btf
> > >   All the different flags and options for different pahole
> > >   versions can be dropped.
> > > 
> > > ---
> > > 
> > > On the other hand, I'm not sure this useful but relatively obscure
> > > feature grants such a compatibility break. Some time ago Ihor
> > > advocated for just having two functions in the kernel, so that BTF
> > > will be generated for both. And I think that someone suggested putting
> > > the fake function to a discard-able section.
> > > This way the whole thing can be done in kernel only.
> > > E.g. it will look like so:
> > > 
> > >   __bpf_kfunc void btf_foo_impl(struct bpf_prog_aux p__implicit)
> > >   { /* real impl here */ }
> > > 
> > >   __bpf_kfunc_proto void btf_foo(void) {}
> > > 
> > > Assuming that __bpf_kfunc_proto hides all the necessary attributes.
> > > Not much boilerplate, and a tad easier to understand where second
> > > prototype comes from, no need to read pahole.
> > 
> > Scheme discussed off-list for new functions with __implicit args:
> > - kernel source code:
> > 
> >     __bpf_kfunc void foo(struct bpf_prog_aux p__implicit)
> > 	BTF_ID_FLAGS(foo, KF_IMPLICIT_ARGS)
> > 
> > - pahole:
> >   - renames foo to foo_impl
> >   - adds bpf-side definition for 'foo' w/o implicit args
> >   vmlinux btf:
> > 
> >     __bpf_kfunc void foo_impl(struct bpf_prog_aux p__implicit);
> >     void foo(void);
> 
> I believe it's the other way around:
>      void foo_impl(struct bpf_prog_aux p__implicit);
>      __bpf_kfunc void foo(void);
> 
> foo() is callable from BPF, but foo_impl() is not.
> But we still want foo_impl() in BTF so that the verifier can find the 
> correct prototype.
> 
> Andrii, please confirm.

Oops, yes, 'foo' gets __bpf_kfunc.

> > - resolve_btfids puts the 'foo' (the one w/o implicit args) id to all
> >   id lists (no changes needed for this, follows from pahole changes);
> > - verifier.c:add_kfunc_call()
> >   - Sees the id of 'foo' and kfunc flags with KF_IMPLICIT_ARGS
> >   - Replaces the id with id of 'foo_impl'.
> > 
> > This will break the following scenario:
> > - new kfunc is added with __implicit arg
> > - kernel is built with old pahole
> > - vmlinux.h is generated for such kernel
> > - bpf program is compiled against such vmlinux.h
> > - attempt to run such program on a new kernel compiled with new pahole
> >   will fail.
> 
> I think you meant "new kernel compiled with old pahole".

No, new kernel compiled with new pahole.
As it will have a different prototype for 'foo' compared to one
encoded in the old program binary 'btf'.

> 
> > 
> > Andrei and Alexei deemed this acceptable.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30 18:26       ` Alan Maguire
@ 2025-10-30 18:42         ` Eduard Zingerman
  2025-10-30 18:46         ` Ihor Solodrai
  1 sibling, 0 replies; 36+ messages in thread
From: Eduard Zingerman @ 2025-10-30 18:42 UTC (permalink / raw)
  To: Alan Maguire, Ihor Solodrai, bpf, andrii, ast
  Cc: dwarves, acme, tj, kernel-team

On Thu, 2025-10-30 at 18:26 +0000, Alan Maguire wrote:

[...]

> Okay so bear with me as this is probably a massive over-simplification.
> It seems like what we want here is a way to establish a relationship
> between the BTF associated with the _impl function and the kfunc-visible
> form (without the implicit arguments), right? Once we have that
> relationship, it's sort of implicit which are the implicit arguments;
> they're the ones the _impl variant has and the non-impl variant doesn't
> have. So to me - and again I'm probably missing a lot - the key thing is
> to establish that relationship between kfunc and kfunc_impl. Couldn't we
> leverage the kernel build machinery around resolve_btf_ids to construct
> these pairwise mappings of BTF ids? That way we keep pahole out of the
> loop (aside from generating BTF for both variants as usual) and
> compatibility issues aren't there as much because resolve_btfids travels
> with the kernel, no changes needed for pahole.
> 
> I'm guessing the above is missing something though?

Andrii does not like having to declare two functions manually in C code:

  __bpf_kfunc foo_impl(p__implicit);
  __bpf_kfunc foo(void);    <-------- don't want to declare this explicitly

But prototypes for both functions are needed in the BTF.
And the only place that can invent new BTFs is pahole.
Imo, that's a bit of an over-complication ¯\_(ツ)_/¯.
  

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30 18:26       ` Alan Maguire
  2025-10-30 18:42         ` Eduard Zingerman
@ 2025-10-30 18:46         ` Ihor Solodrai
  2025-10-30 19:47           ` Andrii Nakryiko
  1 sibling, 1 reply; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-30 18:46 UTC (permalink / raw)
  To: Alan Maguire, Eduard Zingerman, bpf, andrii, ast
  Cc: dwarves, acme, tj, kernel-team

On 10/30/25 11:26 AM, Alan Maguire wrote:
> On 30/10/2025 18:14, Eduard Zingerman wrote:
>> On Wed, 2025-10-29 at 23:11 -0700, Eduard Zingerman wrote:
>>> On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
>>>> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
>>>>
>>>> [...]
>>
>> Scheme discussed off-list for new functions with __implicit args:
>> - kernel source code:
>>
>>     __bpf_kfunc void foo(struct bpf_prog_aux p__implicit)
>> 	BTF_ID_FLAGS(foo, KF_IMPLICIT_ARGS)
>>
>> - pahole:
>>   - renames foo to foo_impl
>>   - adds bpf-side definition for 'foo' w/o implicit args
>>   vmlinux btf:
>>
>>     __bpf_kfunc void foo_impl(struct bpf_prog_aux p__implicit);
>>     void foo(void);
>>
>> - resolve_btfids puts the 'foo' (the one w/o implicit args) id to all
>>   id lists (no changes needed for this, follows from pahole changes);
>> - verifier.c:add_kfunc_call()
>>   - Sees the id of 'foo' and kfunc flags with KF_IMPLICIT_ARGS
>>   - Replaces the id with id of 'foo_impl'.
>>
>> This will break the following scenario:
>> - new kfunc is added with __implicit arg
>> - kernel is built with old pahole
>> - vmlinux.h is generated for such kernel
>> - bpf program is compiled against such vmlinux.h
>> - attempt to run such program on a new kernel compiled with new pahole
>>   will fail.
>>
>> Andrei and Alexei deemed this acceptable.
> 
> Okay so bear with me as this is probably a massive over-simplification.
> It seems like what we want here is a way to establish a relationship
> between the BTF associated with the _impl function and the kfunc-visible
> form (without the implicit arguments), right? Once we have that
> relationship, it's sort of implicit which are the implicit arguments;
> they're the ones the _impl variant has and the non-impl variant doesn't
> have. So to me - and again I'm probably missing a lot - the key thing is
> to establish that relationship between kfunc and kfunc_impl. Couldn't we
> leverage the kernel build machinery around resolve_btf_ids to construct
> these pairwise mappings of BTF ids? That way we keep pahole out of the
> loop (aside from generating BTF for both variants as usual) and
> compatibility issues aren't there as much because resolve_btfids travels
> with the kernel, no changes needed for pahole.

We've had a couple of rounds of back and forth on this.

The reasoning here is that going forward we want to make a kfunc with
implicit arguments easy to define. That is:

    __bpf_kfunc int bpf_kfunc(int arg, struct bpf_prog_aux *aux__impl) {}
    BTF_ID_FLAGS(func, bpf_kfunc, KF_IMPLICIT_ARGS)

That's it.

In order to keep pahole out of the loop, it's necessary to have both
interface and implementation declarations in the kernel. Something
like this:

    __bpf_kfunc_interface int bpf_kfunc(int arg) {}
    __bpf_kfunc int bpf_kfunc_impl(int arg, struct bpf_prog_aux *aux__impl) {}
    BTF_ID_FLAGS(func, bpf_kfunc, KF_IMPLICIT_ARGS)

Which shifts most of the burden of resolving KF_IMPLICIT_ARGS to the
verifier. But then of course both variants will be callable from BPF,
which is another thing we'd like to avoid.

pahole provides an ability to modify BTF only, and make that
bpf_kfunc_impl (almost) invisible to the user, which is great.

The downside is that maintaining backwards compatibility between
kernel, pahole and BPF binaries is difficult.


> 
> I'm guessing the above is missing something though?
> 
> Thanks!
> 
> Alan


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30 18:46         ` Ihor Solodrai
@ 2025-10-30 19:47           ` Andrii Nakryiko
  2025-10-30 20:02             ` Ihor Solodrai
  0 siblings, 1 reply; 36+ messages in thread
From: Andrii Nakryiko @ 2025-10-30 19:47 UTC (permalink / raw)
  To: Ihor Solodrai
  Cc: Alan Maguire, Eduard Zingerman, bpf, andrii, ast, dwarves, acme,
	tj, kernel-team

On Thu, Oct 30, 2025 at 11:46 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>
> On 10/30/25 11:26 AM, Alan Maguire wrote:
> > On 30/10/2025 18:14, Eduard Zingerman wrote:
> >> On Wed, 2025-10-29 at 23:11 -0700, Eduard Zingerman wrote:
> >>> On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
> >>>> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> >>>>
> >>>> [...]
> >>
> >> Scheme discussed off-list for new functions with __implicit args:
> >> - kernel source code:
> >>
> >>     __bpf_kfunc void foo(struct bpf_prog_aux p__implicit)
> >>      BTF_ID_FLAGS(foo, KF_IMPLICIT_ARGS)
> >>
> >> - pahole:
> >>   - renames foo to foo_impl
> >>   - adds bpf-side definition for 'foo' w/o implicit args
> >>   vmlinux btf:
> >>
> >>     __bpf_kfunc void foo_impl(struct bpf_prog_aux p__implicit);
> >>     void foo(void);
> >>
> >> - resolve_btfids puts the 'foo' (the one w/o implicit args) id to all
> >>   id lists (no changes needed for this, follows from pahole changes);
> >> - verifier.c:add_kfunc_call()
> >>   - Sees the id of 'foo' and kfunc flags with KF_IMPLICIT_ARGS
> >>   - Replaces the id with id of 'foo_impl'.
> >>
> >> This will break the following scenario:
> >> - new kfunc is added with __implicit arg
> >> - kernel is built with old pahole
> >> - vmlinux.h is generated for such kernel
> >> - bpf program is compiled against such vmlinux.h
> >> - attempt to run such program on a new kernel compiled with new pahole
> >>   will fail.
> >>
> >> Andrei and Alexei deemed this acceptable.
> >
> > Okay so bear with me as this is probably a massive over-simplification.
> > It seems like what we want here is a way to establish a relationship
> > between the BTF associated with the _impl function and the kfunc-visible
> > form (without the implicit arguments), right? Once we have that
> > relationship, it's sort of implicit which are the implicit arguments;
> > they're the ones the _impl variant has and the non-impl variant doesn't
> > have. So to me - and again I'm probably missing a lot - the key thing is
> > to establish that relationship between kfunc and kfunc_impl. Couldn't we
> > leverage the kernel build machinery around resolve_btf_ids to construct
> > these pairwise mappings of BTF ids? That way we keep pahole out of the
> > loop (aside from generating BTF for both variants as usual) and
> > compatibility issues aren't there as much because resolve_btfids travels
> > with the kernel, no changes needed for pahole.
>
> We've had a couple of rounds of back and forth on this.
>
> The reasoning here is that going forward we want to make a kfunc with
> implicit arguments easy to define. That is:
>
>     __bpf_kfunc int bpf_kfunc(int arg, struct bpf_prog_aux *aux__impl) {}

I don't think we even need __impl suffix for argument name with
KF_IMPLICIT_ARGS, right?

>     BTF_ID_FLAGS(func, bpf_kfunc, KF_IMPLICIT_ARGS)
>
> That's it.
>
> In order to keep pahole out of the loop, it's necessary to have both
> interface and implementation declarations in the kernel. Something
> like this:
>
>     __bpf_kfunc_interface int bpf_kfunc(int arg) {}
>     __bpf_kfunc int bpf_kfunc_impl(int arg, struct bpf_prog_aux *aux__impl) {}
>     BTF_ID_FLAGS(func, bpf_kfunc, KF_IMPLICIT_ARGS)
>
> Which shifts most of the burden of resolving KF_IMPLICIT_ARGS to the
> verifier. But then of course both variants will be callable from BPF,
> which is another thing we'd like to avoid.
>
> pahole provides an ability to modify BTF only, and make that
> bpf_kfunc_impl (almost) invisible to the user, which is great.
>
> The downside is that maintaining backwards compatibility between
> kernel, pahole and BPF binaries is difficult.
>

I think we should differentiate backwards compat for all existing
_impl kfuncs and BPF programs that used to work with them, and then,
separately, what happens going forward with newly added non-_impl
kfuncs with KF_IMPLICIT_ARGS and BPF programs that want to make use of
these _impl-less kfuncs.

For existing _impl kfuncs ("legacy" case), backwards compat is 100%
preserved, even if the kernel was built with an old pahole that
doesn't yet know about KF_IMPLICTI_ARGS. There will be BTF for both
_impl and _impl-less func_protos, all that will work. And BPF programs
are explicitly calling _impl kfuncs. So even if pahole didn't do
anything special for KF_IMPLICIT_ARGS, verifier should work fine, it
will find _impl BTF, etc.

For all new _impl-less kfunc definitions and/or BPF programs that
switch to _impl-less calls, yes, we will document and require that
kernel BTF has to be generated with a newer pahole. We can enforce
that in Kconfig, but it's a bit too strict/too soon as it's irrelevant
and unnecessary for the majority of BPF users that don't care about
_impl-less stuff.

Keep in mind, right now we have 4-5 such _impl special functions, but
going forward we will probably have lots. sched-ext is poised to use
that very extensively throughout a lot of its kfuncs, so requiring
these explicit _impl wrappers just to support older pahole with newer
kernels I think doesn't make sense in the grand scheme of things.
Getting the latest pahole released/packaged/used for kernel build for
distros shouldn't be a big deal at all. It's not really like upgrading
the compiler toolchain at all.

>
> >
> > I'm guessing the above is missing something though?
> >
> > Thanks!
> >
> > Alan
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30 19:47           ` Andrii Nakryiko
@ 2025-10-30 20:02             ` Ihor Solodrai
  2025-10-30 20:38               ` Andrii Nakryiko
  0 siblings, 1 reply; 36+ messages in thread
From: Ihor Solodrai @ 2025-10-30 20:02 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alan Maguire, Eduard Zingerman, bpf, andrii, ast, dwarves, acme,
	tj, kernel-team

On 10/30/25 12:47 PM, Andrii Nakryiko wrote:
> On Thu, Oct 30, 2025 at 11:46 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> On 10/30/25 11:26 AM, Alan Maguire wrote:
>>> On 30/10/2025 18:14, Eduard Zingerman wrote:
>>>> On Wed, 2025-10-29 at 23:11 -0700, Eduard Zingerman wrote:
>>>>> On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
>>>>>> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
>>>>>>
>>>>>> [...]
>>>>
>>>> Scheme discussed off-list for new functions with __implicit args:
>>>> - kernel source code:
>>>>
>>>>     __bpf_kfunc void foo(struct bpf_prog_aux p__implicit)
>>>>      BTF_ID_FLAGS(foo, KF_IMPLICIT_ARGS)
>>>>
>>>> - pahole:
>>>>   - renames foo to foo_impl
>>>>   - adds bpf-side definition for 'foo' w/o implicit args
>>>>   vmlinux btf:
>>>>
>>>>     __bpf_kfunc void foo_impl(struct bpf_prog_aux p__implicit);
>>>>     void foo(void);
>>>>
>>>> - resolve_btfids puts the 'foo' (the one w/o implicit args) id to all
>>>>   id lists (no changes needed for this, follows from pahole changes);
>>>> - verifier.c:add_kfunc_call()
>>>>   - Sees the id of 'foo' and kfunc flags with KF_IMPLICIT_ARGS
>>>>   - Replaces the id with id of 'foo_impl'.
>>>>
>>>> This will break the following scenario:
>>>> - new kfunc is added with __implicit arg
>>>> - kernel is built with old pahole
>>>> - vmlinux.h is generated for such kernel
>>>> - bpf program is compiled against such vmlinux.h
>>>> - attempt to run such program on a new kernel compiled with new pahole
>>>>   will fail.
>>>>
>>>> Andrei and Alexei deemed this acceptable.
>>>
>>> Okay so bear with me as this is probably a massive over-simplification.
>>> It seems like what we want here is a way to establish a relationship
>>> between the BTF associated with the _impl function and the kfunc-visible
>>> form (without the implicit arguments), right? Once we have that
>>> relationship, it's sort of implicit which are the implicit arguments;
>>> they're the ones the _impl variant has and the non-impl variant doesn't
>>> have. So to me - and again I'm probably missing a lot - the key thing is
>>> to establish that relationship between kfunc and kfunc_impl. Couldn't we
>>> leverage the kernel build machinery around resolve_btf_ids to construct
>>> these pairwise mappings of BTF ids? That way we keep pahole out of the
>>> loop (aside from generating BTF for both variants as usual) and
>>> compatibility issues aren't there as much because resolve_btfids travels
>>> with the kernel, no changes needed for pahole.
>>
>> We've had a couple of rounds of back and forth on this.
>>
>> The reasoning here is that going forward we want to make a kfunc with
>> implicit arguments easy to define. That is:
>>
>>     __bpf_kfunc int bpf_kfunc(int arg, struct bpf_prog_aux *aux__impl) {}
> 
> I don't think we even need __impl suffix for argument name with
> KF_IMPLICIT_ARGS, right?

I mentioned options that we discussed before in the cover letter.

Basically, pahole needs to figure out how many arguments to omit *somehow*.
Using a name suffix (aka annotation) seems to be the most flexible way, as
it allows to avoid changes in pahole if/when we add new implicit arg types.

> 
>>     BTF_ID_FLAGS(func, bpf_kfunc, KF_IMPLICIT_ARGS)
>>
>> That's it.
>>
>> In order to keep pahole out of the loop, it's necessary to have both
>> interface and implementation declarations in the kernel. Something
>> like this:
>>
>>     __bpf_kfunc_interface int bpf_kfunc(int arg) {}
>>     __bpf_kfunc int bpf_kfunc_impl(int arg, struct bpf_prog_aux *aux__impl) {}
>>     BTF_ID_FLAGS(func, bpf_kfunc, KF_IMPLICIT_ARGS)
>>
>> Which shifts most of the burden of resolving KF_IMPLICIT_ARGS to the
>> verifier. But then of course both variants will be callable from BPF,
>> which is another thing we'd like to avoid.
>>
>> pahole provides an ability to modify BTF only, and make that
>> bpf_kfunc_impl (almost) invisible to the user, which is great.
>>
>> The downside is that maintaining backwards compatibility between
>> kernel, pahole and BPF binaries is difficult.
>>
> 
> I think we should differentiate backwards compat for all existing
> _impl kfuncs and BPF programs that used to work with them, and then,
> separately, what happens going forward with newly added non-_impl
> kfuncs with KF_IMPLICIT_ARGS and BPF programs that want to make use of
> these _impl-less kfuncs.
> 
> For existing _impl kfuncs ("legacy" case), backwards compat is 100%
> preserved, even if the kernel was built with an old pahole that
> doesn't yet know about KF_IMPLICTI_ARGS. There will be BTF for both
> _impl and _impl-less func_protos, all that will work. And BPF programs
> are explicitly calling _impl kfuncs. So even if pahole didn't do
> anything special for KF_IMPLICIT_ARGS, verifier should work fine, it
> will find _impl BTF, etc.
> 
> For all new _impl-less kfunc definitions and/or BPF programs that
> switch to _impl-less calls, yes, we will document and require that
> kernel BTF has to be generated with a newer pahole. We can enforce
> that in Kconfig, but it's a bit too strict/too soon as it's irrelevant
> and unnecessary for the majority of BPF users that don't care about
> _impl-less stuff.

I think some kind of build-time enforcement will be necessary.
It's *very* easy to unintentionally use old(-er) pahole version
for kernel build, especially when the new version is recent.

> 
> Keep in mind, right now we have 4-5 such _impl special functions, but
> going forward we will probably have lots. sched-ext is poised to use
> that very extensively throughout a lot of its kfuncs, so requiring
> these explicit _impl wrappers just to support older pahole with newer
> kernels I think doesn't make sense in the grand scheme of things.
> Getting the latest pahole released/packaged/used for kernel build for
> distros shouldn't be a big deal at all. It's not really like upgrading
> the compiler toolchain at all.
> 
>>
>>>
>>> I'm guessing the above is missing something though?
>>>
>>> Thanks!
>>>
>>> Alan
>>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH bpf-next v1 0/8] bpf: magic kernel functions
  2025-10-30 20:02             ` Ihor Solodrai
@ 2025-10-30 20:38               ` Andrii Nakryiko
  0 siblings, 0 replies; 36+ messages in thread
From: Andrii Nakryiko @ 2025-10-30 20:38 UTC (permalink / raw)
  To: Ihor Solodrai
  Cc: Alan Maguire, Eduard Zingerman, bpf, andrii, ast, dwarves, acme,
	tj, kernel-team

On Thu, Oct 30, 2025 at 1:02 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>
> On 10/30/25 12:47 PM, Andrii Nakryiko wrote:
> > On Thu, Oct 30, 2025 at 11:46 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
> >>
> >> On 10/30/25 11:26 AM, Alan Maguire wrote:
> >>> On 30/10/2025 18:14, Eduard Zingerman wrote:
> >>>> On Wed, 2025-10-29 at 23:11 -0700, Eduard Zingerman wrote:
> >>>>> On Wed, 2025-10-29 at 17:44 -0700, Eduard Zingerman wrote:
> >>>>>> On Wed, 2025-10-29 at 12:01 -0700, Ihor Solodrai wrote:
> >>>>>>
> >>>>>> [...]
> >>>>
> >>>> Scheme discussed off-list for new functions with __implicit args:
> >>>> - kernel source code:
> >>>>
> >>>>     __bpf_kfunc void foo(struct bpf_prog_aux p__implicit)
> >>>>      BTF_ID_FLAGS(foo, KF_IMPLICIT_ARGS)
> >>>>
> >>>> - pahole:
> >>>>   - renames foo to foo_impl
> >>>>   - adds bpf-side definition for 'foo' w/o implicit args
> >>>>   vmlinux btf:
> >>>>
> >>>>     __bpf_kfunc void foo_impl(struct bpf_prog_aux p__implicit);
> >>>>     void foo(void);
> >>>>
> >>>> - resolve_btfids puts the 'foo' (the one w/o implicit args) id to all
> >>>>   id lists (no changes needed for this, follows from pahole changes);
> >>>> - verifier.c:add_kfunc_call()
> >>>>   - Sees the id of 'foo' and kfunc flags with KF_IMPLICIT_ARGS
> >>>>   - Replaces the id with id of 'foo_impl'.
> >>>>
> >>>> This will break the following scenario:
> >>>> - new kfunc is added with __implicit arg
> >>>> - kernel is built with old pahole
> >>>> - vmlinux.h is generated for such kernel
> >>>> - bpf program is compiled against such vmlinux.h
> >>>> - attempt to run such program on a new kernel compiled with new pahole
> >>>>   will fail.
> >>>>
> >>>> Andrei and Alexei deemed this acceptable.
> >>>
> >>> Okay so bear with me as this is probably a massive over-simplification.
> >>> It seems like what we want here is a way to establish a relationship
> >>> between the BTF associated with the _impl function and the kfunc-visible
> >>> form (without the implicit arguments), right? Once we have that
> >>> relationship, it's sort of implicit which are the implicit arguments;
> >>> they're the ones the _impl variant has and the non-impl variant doesn't
> >>> have. So to me - and again I'm probably missing a lot - the key thing is
> >>> to establish that relationship between kfunc and kfunc_impl. Couldn't we
> >>> leverage the kernel build machinery around resolve_btf_ids to construct
> >>> these pairwise mappings of BTF ids? That way we keep pahole out of the
> >>> loop (aside from generating BTF for both variants as usual) and
> >>> compatibility issues aren't there as much because resolve_btfids travels
> >>> with the kernel, no changes needed for pahole.
> >>
> >> We've had a couple of rounds of back and forth on this.
> >>
> >> The reasoning here is that going forward we want to make a kfunc with
> >> implicit arguments easy to define. That is:
> >>
> >>     __bpf_kfunc int bpf_kfunc(int arg, struct bpf_prog_aux *aux__impl) {}
> >
> > I don't think we even need __impl suffix for argument name with
> > KF_IMPLICIT_ARGS, right?
>
> I mentioned options that we discussed before in the cover letter.
>
> Basically, pahole needs to figure out how many arguments to omit *somehow*.
> Using a name suffix (aka annotation) seems to be the most flexible way, as
> it allows to avoid changes in pahole if/when we add new implicit arg types.


Ah, right, I forgot we discussed this. Ok, arg suffix isn't too bad.

>
> >
> >>     BTF_ID_FLAGS(func, bpf_kfunc, KF_IMPLICIT_ARGS)
> >>
> >> That's it.
> >>
> >> In order to keep pahole out of the loop, it's necessary to have both
> >> interface and implementation declarations in the kernel. Something
> >> like this:
> >>
> >>     __bpf_kfunc_interface int bpf_kfunc(int arg) {}
> >>     __bpf_kfunc int bpf_kfunc_impl(int arg, struct bpf_prog_aux *aux__impl) {}
> >>     BTF_ID_FLAGS(func, bpf_kfunc, KF_IMPLICIT_ARGS)
> >>
> >> Which shifts most of the burden of resolving KF_IMPLICIT_ARGS to the
> >> verifier. But then of course both variants will be callable from BPF,
> >> which is another thing we'd like to avoid.
> >>
> >> pahole provides an ability to modify BTF only, and make that
> >> bpf_kfunc_impl (almost) invisible to the user, which is great.
> >>
> >> The downside is that maintaining backwards compatibility between
> >> kernel, pahole and BPF binaries is difficult.
> >>
> >
> > I think we should differentiate backwards compat for all existing
> > _impl kfuncs and BPF programs that used to work with them, and then,
> > separately, what happens going forward with newly added non-_impl
> > kfuncs with KF_IMPLICIT_ARGS and BPF programs that want to make use of
> > these _impl-less kfuncs.
> >
> > For existing _impl kfuncs ("legacy" case), backwards compat is 100%
> > preserved, even if the kernel was built with an old pahole that
> > doesn't yet know about KF_IMPLICTI_ARGS. There will be BTF for both
> > _impl and _impl-less func_protos, all that will work. And BPF programs
> > are explicitly calling _impl kfuncs. So even if pahole didn't do
> > anything special for KF_IMPLICIT_ARGS, verifier should work fine, it
> > will find _impl BTF, etc.
> >
> > For all new _impl-less kfunc definitions and/or BPF programs that
> > switch to _impl-less calls, yes, we will document and require that
> > kernel BTF has to be generated with a newer pahole. We can enforce
> > that in Kconfig, but it's a bit too strict/too soon as it's irrelevant
> > and unnecessary for the majority of BPF users that don't care about
> > _impl-less stuff.
>
> I think some kind of build-time enforcement will be necessary.
> It's *very* easy to unintentionally use old(-er) pahole version
> for kernel build, especially when the new version is recent.
>

I'd try to avoid failing kernel build if pahole is not recent enough
for this feature.

How about we teach resolve_btfids() to check whether kernel BTF is
"conformant" (i.e., it has xxx and xxx_impl func_protos, and xxx
doesn't have implicit args, while _impl has implicit args), and if
it's not conformant, we just effectively make such _impl-less kfunc
not a kfunc. Would resolving such BTF_ID to zero do the trick?

> >
> > Keep in mind, right now we have 4-5 such _impl special functions, but
> > going forward we will probably have lots. sched-ext is poised to use
> > that very extensively throughout a lot of its kfuncs, so requiring
> > these explicit _impl wrappers just to support older pahole with newer
> > kernels I think doesn't make sense in the grand scheme of things.
> > Getting the latest pahole released/packaged/used for kernel build for
> > distros shouldn't be a big deal at all. It's not really like upgrading
> > the compiler toolchain at all.
> >
> >>
> >>>
> >>> I'm guessing the above is missing something though?
> >>>
> >>> Thanks!
> >>>
> >>> Alan
> >>
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2025-10-30 20:39 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29 19:01 [PATCH bpf-next v1 0/8] bpf: magic kernel functions Ihor Solodrai
2025-10-29 19:01 ` [PATCH bpf-next v1 1/8] bpf: Add BTF_ID_LIST_END and BTF_ID_LIST_SIZE macros Ihor Solodrai
2025-10-29 19:41   ` bot+bpf-ci
2025-10-29 20:44     ` Ihor Solodrai
2025-10-29 23:54   ` Eduard Zingerman
2025-10-29 19:01 ` [PATCH bpf-next v1 2/8] bpf: Refactor btf_kfunc_id_set_contains Ihor Solodrai
2025-10-29 23:55   ` Eduard Zingerman
2025-10-29 19:01 ` [PATCH bpf-next v1 3/8] bpf: Support for kfuncs with KF_MAGIC_ARGS Ihor Solodrai
2025-10-29 19:41   ` bot+bpf-ci
2025-10-29 20:49     ` Ihor Solodrai
2025-10-29 23:59       ` Eduard Zingerman
2025-10-29 23:54   ` Eduard Zingerman
2025-10-30  0:03     ` Alexei Starovoitov
2025-10-30 16:31     ` Ihor Solodrai
2025-10-30 17:26       ` Eduard Zingerman
2025-10-30 10:24   ` kernel test robot
2025-10-30 11:58   ` kernel test robot
2025-10-30 13:54   ` kernel test robot
2025-10-29 19:01 ` [PATCH bpf-next v1 4/8] bpf: Support __magic prog_aux arguments for kfuncs Ihor Solodrai
2025-10-29 19:01 ` [PATCH bpf-next v1 5/8] bpf: Re-define bpf_wq_set_callback as magic kfunc Ihor Solodrai
2025-10-30  0:16   ` Eduard Zingerman
2025-10-29 19:01 ` [PATCH bpf-next v1 6/8] bpf,docs: Document KF_MAGIC_ARGS flag and __magic annotation Ihor Solodrai
2025-10-30  0:21   ` Eduard Zingerman
2025-10-29 19:01 ` [PATCH bpf-next v1 7/8] bpf: Re-define bpf_task_work_schedule_* kfuncs as magic Ihor Solodrai
2025-10-29 19:01 ` [PATCH bpf-next v1 8/8] bpf: Re-define bpf_stream_vprintk as a magic kfunc Ihor Solodrai
2025-10-30  0:44 ` [PATCH bpf-next v1 0/8] bpf: magic kernel functions Eduard Zingerman
2025-10-30  6:11   ` Eduard Zingerman
2025-10-30 18:14     ` Eduard Zingerman
2025-10-30 18:24       ` Ihor Solodrai
2025-10-30 18:37         ` Eduard Zingerman
2025-10-30 18:26       ` Alan Maguire
2025-10-30 18:42         ` Eduard Zingerman
2025-10-30 18:46         ` Ihor Solodrai
2025-10-30 19:47           ` Andrii Nakryiko
2025-10-30 20:02             ` Ihor Solodrai
2025-10-30 20:38               ` Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox