* [PATCH bpf-next] bpf: Use kallsyms to find the function name of a struct_ops's stub function
@ 2025-01-27 22:27 Martin KaFai Lau
2025-01-29 18:15 ` Amery Hung
2025-01-30 2:57 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Martin KaFai Lau @ 2025-01-27 22:27 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Tejun Heo, Benjamin Tissoires, Yonghong Song, Amery Hung
From: Martin KaFai Lau <martin.lau@kernel.org>
In commit 1611603537a4 ("bpf: Create argument information for nullable arguments."),
it introduced a "__nullable" tagging at the argument name of a
stub function. Some background on the commit:
it requires to tag the stub function instead of directly tagging
the "ops" of a struct. This is because the btf func_proto of the "ops"
does not have the argument name and the "__nullable" is tagged at
the argument name.
To find the stub function of a "ops", it currently relies on a naming
convention on the stub function "st_ops__ops_name".
e.g. tcp_congestion_ops__ssthresh. However, the new kernel
sub system implementing bpf_struct_ops have missed this and
have been surprised that the "__nullable" and the to-be-landed
"__ref" tagging was not effective.
One option would be to give a warning whenever the stub function does
not follow the naming convention, regardless if it requires arg tagging
or not.
Instead, this patch uses the kallsyms_lookup approach and removes
the requirement on the naming convention. The st_ops->cfi_stubs has
all the stub function kernel addresses. kallsyms_lookup() is used to
lookup the function name. With the function name, BTF can be used to
find the BTF func_proto. The existing "__nullable" arg name searching
logic will then fall through.
One notable change is,
if it failed in kallsyms_lookup or it failed in looking up the stub
function name from the BTF, the bpf_struct_ops registration will fail.
This is different from the previous behavior that it silently ignored
the "st_ops__ops_name" function not found error.
The "tcp_congestion_ops", "sched_ext_ops", and "hid_bpf_ops" can still be
registered successfully after this patch. There is struct_ops_maybe_null
selftest to cover the "__nullable" tagging.
Other minor changes:
1. Removed the "%s__%s" format from the pr_warn because the naming
convention is removed.
2. The existing bpf_struct_ops_supported() is also moved earlier
because prepare_arg_info needs to use it to decide if the
stub function is NULL before calling the prepare_arg_info.
Cc: Tejun Heo <tj@kernel.org>
Cc: Benjamin Tissoires <bentiss@kernel.org>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Amery Hung <ameryhung@gmail.com>
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
---
kernel/bpf/bpf_struct_ops.c | 98 +++++++++++++++++--------------------
1 file changed, 44 insertions(+), 54 deletions(-)
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 040fb1cd840b..9b7f3b9c5262 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -146,39 +146,6 @@ void bpf_struct_ops_image_free(void *image)
}
#define MAYBE_NULL_SUFFIX "__nullable"
-#define MAX_STUB_NAME 128
-
-/* Return the type info of a stub function, if it exists.
- *
- * The name of a stub function is made up of the name of the struct_ops and
- * the name of the function pointer member, separated by "__". For example,
- * if the struct_ops type is named "foo_ops" and the function pointer
- * member is named "bar", the stub function name would be "foo_ops__bar".
- */
-static const struct btf_type *
-find_stub_func_proto(const struct btf *btf, const char *st_op_name,
- const char *member_name)
-{
- char stub_func_name[MAX_STUB_NAME];
- const struct btf_type *func_type;
- s32 btf_id;
- int cp;
-
- cp = snprintf(stub_func_name, MAX_STUB_NAME, "%s__%s",
- st_op_name, member_name);
- if (cp >= MAX_STUB_NAME) {
- pr_warn("Stub function name too long\n");
- return NULL;
- }
- btf_id = btf_find_by_name_kind(btf, stub_func_name, BTF_KIND_FUNC);
- if (btf_id < 0)
- return NULL;
- func_type = btf_type_by_id(btf, btf_id);
- if (!func_type)
- return NULL;
-
- return btf_type_by_id(btf, func_type->type); /* FUNC_PROTO */
-}
/* Prepare argument info for every nullable argument of a member of a
* struct_ops type.
@@ -203,27 +170,42 @@ find_stub_func_proto(const struct btf *btf, const char *st_op_name,
static int prepare_arg_info(struct btf *btf,
const char *st_ops_name,
const char *member_name,
- const struct btf_type *func_proto,
+ const struct btf_type *func_proto, void *stub_func_addr,
struct bpf_struct_ops_arg_info *arg_info)
{
const struct btf_type *stub_func_proto, *pointed_type;
const struct btf_param *stub_args, *args;
struct bpf_ctx_arg_aux *info, *info_buf;
u32 nargs, arg_no, info_cnt = 0;
+ char ksym[KSYM_SYMBOL_LEN];
+ const char *stub_fname;
+ s32 stub_func_id;
u32 arg_btf_id;
int offset;
- stub_func_proto = find_stub_func_proto(btf, st_ops_name, member_name);
- if (!stub_func_proto)
- return 0;
+ stub_fname = kallsyms_lookup((unsigned long)stub_func_addr, NULL, NULL, NULL, ksym);
+ if (!stub_fname) {
+ pr_warn("Cannot find the stub function name for the %s in struct %s\n",
+ member_name, st_ops_name);
+ return -ENOENT;
+ }
+
+ stub_func_id = btf_find_by_name_kind(btf, stub_fname, BTF_KIND_FUNC);
+ if (stub_func_id < 0) {
+ pr_warn("Cannot find the stub function %s in btf\n", stub_fname);
+ return -ENOENT;
+ }
+
+ stub_func_proto = btf_type_by_id(btf, stub_func_id);
+ stub_func_proto = btf_type_by_id(btf, stub_func_proto->type);
/* Check if the number of arguments of the stub function is the same
* as the number of arguments of the function pointer.
*/
nargs = btf_type_vlen(func_proto);
if (nargs != btf_type_vlen(stub_func_proto)) {
- pr_warn("the number of arguments of the stub function %s__%s does not match the number of arguments of the member %s of struct %s\n",
- st_ops_name, member_name, member_name, st_ops_name);
+ pr_warn("the number of arguments of the stub function %s does not match the number of arguments of the member %s of struct %s\n",
+ stub_fname, member_name, st_ops_name);
return -EINVAL;
}
@@ -253,21 +235,21 @@ static int prepare_arg_info(struct btf *btf,
&arg_btf_id);
if (!pointed_type ||
!btf_type_is_struct(pointed_type)) {
- pr_warn("stub function %s__%s has %s tagging to an unsupported type\n",
- st_ops_name, member_name, MAYBE_NULL_SUFFIX);
+ pr_warn("stub function %s has %s tagging to an unsupported type\n",
+ stub_fname, MAYBE_NULL_SUFFIX);
goto err_out;
}
offset = btf_ctx_arg_offset(btf, func_proto, arg_no);
if (offset < 0) {
- pr_warn("stub function %s__%s has an invalid trampoline ctx offset for arg#%u\n",
- st_ops_name, member_name, arg_no);
+ pr_warn("stub function %s has an invalid trampoline ctx offset for arg#%u\n",
+ stub_fname, arg_no);
goto err_out;
}
if (args[arg_no].type != stub_args[arg_no].type) {
- pr_warn("arg#%u type in stub function %s__%s does not match with its original func_proto\n",
- arg_no, st_ops_name, member_name);
+ pr_warn("arg#%u type in stub function %s does not match with its original func_proto\n",
+ arg_no, stub_fname);
goto err_out;
}
@@ -324,6 +306,13 @@ static bool is_module_member(const struct btf *btf, u32 id)
return !strcmp(btf_name_by_offset(btf, t->name_off), "module");
}
+int bpf_struct_ops_supported(const struct bpf_struct_ops *st_ops, u32 moff)
+{
+ void *func_ptr = *(void **)(st_ops->cfi_stubs + moff);
+
+ return func_ptr ? 0 : -ENOTSUPP;
+}
+
int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
struct btf *btf,
struct bpf_verifier_log *log)
@@ -387,7 +376,10 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
for_each_member(i, t, member) {
const struct btf_type *func_proto;
+ void **stub_func_addr;
+ u32 moff;
+ moff = __btf_member_bit_offset(t, member) / 8;
mname = btf_name_by_offset(btf, member->name_off);
if (!*mname) {
pr_warn("anon member in struct %s is not supported\n",
@@ -413,7 +405,11 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
func_proto = btf_type_resolve_func_ptr(btf,
member->type,
NULL);
- if (!func_proto)
+
+ /* The member is not a function pointer or
+ * the function pointer is not supported.
+ */
+ if (!func_proto || bpf_struct_ops_supported(st_ops, moff))
continue;
if (btf_distill_func_proto(log, btf,
@@ -425,8 +421,9 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
goto errout;
}
+ stub_func_addr = *(void **)(st_ops->cfi_stubs + moff);
err = prepare_arg_info(btf, st_ops->name, mname,
- func_proto,
+ func_proto, stub_func_addr,
arg_info + i);
if (err)
goto errout;
@@ -1152,13 +1149,6 @@ void bpf_struct_ops_put(const void *kdata)
bpf_map_put(&st_map->map);
}
-int bpf_struct_ops_supported(const struct bpf_struct_ops *st_ops, u32 moff)
-{
- void *func_ptr = *(void **)(st_ops->cfi_stubs + moff);
-
- return func_ptr ? 0 : -ENOTSUPP;
-}
-
static bool bpf_struct_ops_valid_to_reg(struct bpf_map *map)
{
struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
--
2.43.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Use kallsyms to find the function name of a struct_ops's stub function
2025-01-27 22:27 [PATCH bpf-next] bpf: Use kallsyms to find the function name of a struct_ops's stub function Martin KaFai Lau
@ 2025-01-29 18:15 ` Amery Hung
2025-01-30 2:57 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Amery Hung @ 2025-01-29 18:15 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Tejun Heo, Benjamin Tissoires, Yonghong Song
On Mon, Jan 27, 2025 at 2:27 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> From: Martin KaFai Lau <martin.lau@kernel.org>
>
> In commit 1611603537a4 ("bpf: Create argument information for nullable arguments."),
> it introduced a "__nullable" tagging at the argument name of a
> stub function. Some background on the commit:
> it requires to tag the stub function instead of directly tagging
> the "ops" of a struct. This is because the btf func_proto of the "ops"
> does not have the argument name and the "__nullable" is tagged at
> the argument name.
>
> To find the stub function of a "ops", it currently relies on a naming
> convention on the stub function "st_ops__ops_name".
> e.g. tcp_congestion_ops__ssthresh. However, the new kernel
> sub system implementing bpf_struct_ops have missed this and
> have been surprised that the "__nullable" and the to-be-landed
> "__ref" tagging was not effective.
>
> One option would be to give a warning whenever the stub function does
> not follow the naming convention, regardless if it requires arg tagging
> or not.
>
> Instead, this patch uses the kallsyms_lookup approach and removes
> the requirement on the naming convention. The st_ops->cfi_stubs has
> all the stub function kernel addresses. kallsyms_lookup() is used to
> lookup the function name. With the function name, BTF can be used to
> find the BTF func_proto. The existing "__nullable" arg name searching
> logic will then fall through.
>
> One notable change is,
> if it failed in kallsyms_lookup or it failed in looking up the stub
> function name from the BTF, the bpf_struct_ops registration will fail.
> This is different from the previous behavior that it silently ignored
> the "st_ops__ops_name" function not found error.
>
> The "tcp_congestion_ops", "sched_ext_ops", and "hid_bpf_ops" can still be
> registered successfully after this patch. There is struct_ops_maybe_null
> selftest to cover the "__nullable" tagging.
>
The patch looks good to me. Also tested with selftests in the qdisc
set and they passed.
Reviewed-by: Amery Hung <ameryhung@gmail.com>
> Other minor changes:
> 1. Removed the "%s__%s" format from the pr_warn because the naming
> convention is removed.
> 2. The existing bpf_struct_ops_supported() is also moved earlier
> because prepare_arg_info needs to use it to decide if the
> stub function is NULL before calling the prepare_arg_info.
>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Benjamin Tissoires <bentiss@kernel.org>
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: Amery Hung <ameryhung@gmail.com>
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> ---
> kernel/bpf/bpf_struct_ops.c | 98 +++++++++++++++++--------------------
> 1 file changed, 44 insertions(+), 54 deletions(-)
>
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 040fb1cd840b..9b7f3b9c5262 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -146,39 +146,6 @@ void bpf_struct_ops_image_free(void *image)
> }
>
> #define MAYBE_NULL_SUFFIX "__nullable"
> -#define MAX_STUB_NAME 128
> -
> -/* Return the type info of a stub function, if it exists.
> - *
> - * The name of a stub function is made up of the name of the struct_ops and
> - * the name of the function pointer member, separated by "__". For example,
> - * if the struct_ops type is named "foo_ops" and the function pointer
> - * member is named "bar", the stub function name would be "foo_ops__bar".
> - */
> -static const struct btf_type *
> -find_stub_func_proto(const struct btf *btf, const char *st_op_name,
> - const char *member_name)
> -{
> - char stub_func_name[MAX_STUB_NAME];
> - const struct btf_type *func_type;
> - s32 btf_id;
> - int cp;
> -
> - cp = snprintf(stub_func_name, MAX_STUB_NAME, "%s__%s",
> - st_op_name, member_name);
> - if (cp >= MAX_STUB_NAME) {
> - pr_warn("Stub function name too long\n");
> - return NULL;
> - }
> - btf_id = btf_find_by_name_kind(btf, stub_func_name, BTF_KIND_FUNC);
> - if (btf_id < 0)
> - return NULL;
> - func_type = btf_type_by_id(btf, btf_id);
> - if (!func_type)
> - return NULL;
> -
> - return btf_type_by_id(btf, func_type->type); /* FUNC_PROTO */
> -}
>
> /* Prepare argument info for every nullable argument of a member of a
> * struct_ops type.
> @@ -203,27 +170,42 @@ find_stub_func_proto(const struct btf *btf, const char *st_op_name,
> static int prepare_arg_info(struct btf *btf,
> const char *st_ops_name,
> const char *member_name,
> - const struct btf_type *func_proto,
> + const struct btf_type *func_proto, void *stub_func_addr,
> struct bpf_struct_ops_arg_info *arg_info)
> {
> const struct btf_type *stub_func_proto, *pointed_type;
> const struct btf_param *stub_args, *args;
> struct bpf_ctx_arg_aux *info, *info_buf;
> u32 nargs, arg_no, info_cnt = 0;
> + char ksym[KSYM_SYMBOL_LEN];
> + const char *stub_fname;
> + s32 stub_func_id;
> u32 arg_btf_id;
> int offset;
>
> - stub_func_proto = find_stub_func_proto(btf, st_ops_name, member_name);
> - if (!stub_func_proto)
> - return 0;
> + stub_fname = kallsyms_lookup((unsigned long)stub_func_addr, NULL, NULL, NULL, ksym);
> + if (!stub_fname) {
> + pr_warn("Cannot find the stub function name for the %s in struct %s\n",
> + member_name, st_ops_name);
> + return -ENOENT;
> + }
> +
> + stub_func_id = btf_find_by_name_kind(btf, stub_fname, BTF_KIND_FUNC);
> + if (stub_func_id < 0) {
> + pr_warn("Cannot find the stub function %s in btf\n", stub_fname);
> + return -ENOENT;
> + }
> +
> + stub_func_proto = btf_type_by_id(btf, stub_func_id);
> + stub_func_proto = btf_type_by_id(btf, stub_func_proto->type);
>
> /* Check if the number of arguments of the stub function is the same
> * as the number of arguments of the function pointer.
> */
> nargs = btf_type_vlen(func_proto);
> if (nargs != btf_type_vlen(stub_func_proto)) {
> - pr_warn("the number of arguments of the stub function %s__%s does not match the number of arguments of the member %s of struct %s\n",
> - st_ops_name, member_name, member_name, st_ops_name);
> + pr_warn("the number of arguments of the stub function %s does not match the number of arguments of the member %s of struct %s\n",
> + stub_fname, member_name, st_ops_name);
> return -EINVAL;
> }
>
> @@ -253,21 +235,21 @@ static int prepare_arg_info(struct btf *btf,
> &arg_btf_id);
> if (!pointed_type ||
> !btf_type_is_struct(pointed_type)) {
> - pr_warn("stub function %s__%s has %s tagging to an unsupported type\n",
> - st_ops_name, member_name, MAYBE_NULL_SUFFIX);
> + pr_warn("stub function %s has %s tagging to an unsupported type\n",
> + stub_fname, MAYBE_NULL_SUFFIX);
> goto err_out;
> }
>
> offset = btf_ctx_arg_offset(btf, func_proto, arg_no);
> if (offset < 0) {
> - pr_warn("stub function %s__%s has an invalid trampoline ctx offset for arg#%u\n",
> - st_ops_name, member_name, arg_no);
> + pr_warn("stub function %s has an invalid trampoline ctx offset for arg#%u\n",
> + stub_fname, arg_no);
> goto err_out;
> }
>
> if (args[arg_no].type != stub_args[arg_no].type) {
> - pr_warn("arg#%u type in stub function %s__%s does not match with its original func_proto\n",
> - arg_no, st_ops_name, member_name);
> + pr_warn("arg#%u type in stub function %s does not match with its original func_proto\n",
> + arg_no, stub_fname);
> goto err_out;
> }
>
> @@ -324,6 +306,13 @@ static bool is_module_member(const struct btf *btf, u32 id)
> return !strcmp(btf_name_by_offset(btf, t->name_off), "module");
> }
>
> +int bpf_struct_ops_supported(const struct bpf_struct_ops *st_ops, u32 moff)
> +{
> + void *func_ptr = *(void **)(st_ops->cfi_stubs + moff);
> +
> + return func_ptr ? 0 : -ENOTSUPP;
> +}
> +
> int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
> struct btf *btf,
> struct bpf_verifier_log *log)
> @@ -387,7 +376,10 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
>
> for_each_member(i, t, member) {
> const struct btf_type *func_proto;
> + void **stub_func_addr;
> + u32 moff;
>
> + moff = __btf_member_bit_offset(t, member) / 8;
> mname = btf_name_by_offset(btf, member->name_off);
> if (!*mname) {
> pr_warn("anon member in struct %s is not supported\n",
> @@ -413,7 +405,11 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
> func_proto = btf_type_resolve_func_ptr(btf,
> member->type,
> NULL);
> - if (!func_proto)
> +
> + /* The member is not a function pointer or
> + * the function pointer is not supported.
> + */
> + if (!func_proto || bpf_struct_ops_supported(st_ops, moff))
> continue;
>
> if (btf_distill_func_proto(log, btf,
> @@ -425,8 +421,9 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
> goto errout;
> }
>
> + stub_func_addr = *(void **)(st_ops->cfi_stubs + moff);
> err = prepare_arg_info(btf, st_ops->name, mname,
> - func_proto,
> + func_proto, stub_func_addr,
> arg_info + i);
> if (err)
> goto errout;
> @@ -1152,13 +1149,6 @@ void bpf_struct_ops_put(const void *kdata)
> bpf_map_put(&st_map->map);
> }
>
> -int bpf_struct_ops_supported(const struct bpf_struct_ops *st_ops, u32 moff)
> -{
> - void *func_ptr = *(void **)(st_ops->cfi_stubs + moff);
> -
> - return func_ptr ? 0 : -ENOTSUPP;
> -}
> -
> static bool bpf_struct_ops_valid_to_reg(struct bpf_map *map)
> {
> struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Use kallsyms to find the function name of a struct_ops's stub function
2025-01-27 22:27 [PATCH bpf-next] bpf: Use kallsyms to find the function name of a struct_ops's stub function Martin KaFai Lau
2025-01-29 18:15 ` Amery Hung
@ 2025-01-30 2:57 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-01-30 2:57 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, ast, andrii, daniel, kernel-team, tj, bentiss, yonghong.song,
ameryhung
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Mon, 27 Jan 2025 14:27:19 -0800 you wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
>
> In commit 1611603537a4 ("bpf: Create argument information for nullable arguments."),
> it introduced a "__nullable" tagging at the argument name of a
> stub function. Some background on the commit:
> it requires to tag the stub function instead of directly tagging
> the "ops" of a struct. This is because the btf func_proto of the "ops"
> does not have the argument name and the "__nullable" is tagged at
> the argument name.
>
> [...]
Here is the summary with links:
- [bpf-next] bpf: Use kallsyms to find the function name of a struct_ops's stub function
https://git.kernel.org/bpf/bpf-next/c/9af5c78155a0
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-30 2:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-27 22:27 [PATCH bpf-next] bpf: Use kallsyms to find the function name of a struct_ops's stub function Martin KaFai Lau
2025-01-29 18:15 ` Amery Hung
2025-01-30 2:57 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox