[RFC bpf-next 0/2] bpf: add a kallsyms BPF iterator

[RFC bpf-next 0/2] bpf: add a kallsyms BPF iterator

[Alan Maguire]

a kallsyms BPF iterator would be useful as it would allow more flexible
iteractions with kernel symbols than are currently supported; it could
for example create more efficient map representations for lookup,
speed up symbol resolution etc.

The idea was initially discussed here [1].

[1] https://lore.kernel.org/all/YjRPZj6Z8vuLeEZo@krava/

Alan Maguire (2):
  bpf: add a kallsyms BPF iterator
  selftests/bpf: add a kallsyms iter subtest

 kernel/kallsyms.c                                  | 93 ++++++++++++++++++++++
 tools/testing/selftests/bpf/prog_tests/bpf_iter.c  | 16 ++++
 .../selftests/bpf/progs/bpf_iter_kallsyms.c        | 71 +++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_kallsyms.c

-- 
1.8.3.1

[RFC bpf-next 1/2] bpf: add a kallsyms BPF iterator

[Alan Maguire]

add a "kallsyms" iterator which provides access to a "struct kallsym_iter"
for each symbol.  Intent is to support more flexible symbol parsing
as discussed in [1].

[1] https://lore.kernel.org/all/YjRPZj6Z8vuLeEZo@krava/

Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 kernel/kallsyms.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index fbdf8d3..ffaf464 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -30,6 +30,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/bsearch.h>
+#include <linux/btf_ids.h>
 
 /*
  * These will be re-linked against their real values
@@ -799,6 +800,95 @@ static int s_show(struct seq_file *m, void *p)
 	.show = s_show
 };
 
+#ifdef CONFIG_BPF_SYSCALL
+
+struct bpf_iter__kallsyms {
+	__bpf_md_ptr(struct bpf_iter_meta *, meta);
+	__bpf_md_ptr(struct kallsym_iter *, kallsym_iter);
+};
+
+static int s_prog_seq_show(struct seq_file *m, bool in_stop)
+{
+	struct bpf_iter__kallsyms ctx;
+	struct bpf_iter_meta meta;
+	struct bpf_prog *prog;
+
+	meta.seq = m;
+	prog = bpf_iter_get_info(&meta, in_stop);
+	if (!prog)
+		return 0;
+
+	ctx.meta = &meta;
+	ctx.kallsym_iter = m ? m->private : NULL;
+	return bpf_iter_run_prog(prog, &ctx);
+}
+
+static int bpf_iter_s_seq_show(struct seq_file *m, void *p)
+{
+	return s_prog_seq_show(m, false);
+}
+
+static void bpf_iter_s_seq_stop(struct seq_file *m, void *p)
+{
+	if (!p)
+		(void) s_prog_seq_show(m, true);
+	else
+		s_stop(m, p);
+}
+
+static const struct seq_operations bpf_iter_kallsyms_ops = {
+	.start = s_start,
+	.next = s_next,
+	.stop = bpf_iter_s_seq_stop,
+	.show = bpf_iter_s_seq_show,
+};
+
+#if defined(CONFIG_PROC_FS)
+
+static int bpf_iter_s_init(void *priv_data, struct bpf_iter_aux_info *aux)
+{
+	struct kallsym_iter *iter = priv_data;
+
+	reset_iter(iter, 0);
+
+	iter->show_value = true;
+
+	return 0;
+}
+
+DEFINE_BPF_ITER_FUNC(kallsyms, struct bpf_iter_meta *meta, struct kallsym_iter *kallsym_iter)
+
+static const struct bpf_iter_seq_info kallsyms_iter_seq_info = {
+	.seq_ops		= &bpf_iter_kallsyms_ops,
+	.init_seq_private	= bpf_iter_s_init,
+	.fini_seq_private	= NULL,
+	.seq_priv_size		= sizeof(struct kallsym_iter),
+};
+
+static struct bpf_iter_reg kallsyms_iter_reg_info = {
+	.target                 = "kallsyms",
+	.ctx_arg_info_size	= 1,
+	.ctx_arg_info		= {
+		{ offsetof(struct bpf_iter__kallsyms, kallsym_iter),
+		  PTR_TO_BTF_ID_OR_NULL },
+	},
+	.seq_info		= &kallsyms_iter_seq_info,
+};
+
+BTF_ID_LIST(btf_kallsym_iter_id)
+BTF_ID(struct, kallsym_iter)
+
+static void __init bpf_kallsyms_iter_register(void)
+{
+	kallsyms_iter_reg_info.ctx_arg_info[0].btf_id = *btf_kallsym_iter_id;
+	if (bpf_iter_reg_target(&kallsyms_iter_reg_info))
+		pr_warn("Warning: could not register bpf kallsyms iterator\n");
+}
+
+#endif /* CONFIG_PROC_FS */
+
+#endif /* CONFIG_BPF_SYSCALL */
+
 static inline int kallsyms_for_perf(void)
 {
 #ifdef CONFIG_PERF_EVENTS
@@ -885,6 +975,9 @@ const char *kdb_walk_kallsyms(loff_t *pos)
 static int __init kallsyms_init(void)
 {
 	proc_create("kallsyms", 0444, NULL, &kallsyms_proc_ops);
+#if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_PROC_FS)
+	bpf_kallsyms_iter_register();
+#endif
 	return 0;
 }
 device_initcall(kallsyms_init);
-- 
1.8.3.1

[RFC bpf-next 2/2] selftests/bpf: add a kallsyms iter subtest

[Alan Maguire]

add subtest verifying BPF kallsyms iter behaviour.  The BPF kallsyms
iter program shows an example of dumping a format different to
kallsyms.  It adds KIND and MAX_SIZE fields which represent the
kind of symbol (core kernel, module, ftrace, bpf, or kprobe) and
the maximum size the symbol can be.  The latter is calculated from
the difference between current symbol value and the next symbol
value.

The key benefit for this iterator will likely be supporting in-kernel
data-gathering rather than dumping symbol details to userspace and
parsing the results.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/testing/selftests/bpf/prog_tests/bpf_iter.c  | 16 +++++
 .../selftests/bpf/progs/bpf_iter_kallsyms.c        | 71 ++++++++++++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_kallsyms.c

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 7ff5fa9..1197a58 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -27,6 +27,7 @@
 #include "bpf_iter_test_kern5.skel.h"
 #include "bpf_iter_test_kern6.skel.h"
 #include "bpf_iter_bpf_link.skel.h"
+#include "bpf_iter_kallsyms.skel.h"
 
 static int duration;
 
@@ -1120,6 +1121,19 @@ static void test_link_iter(void)
 	bpf_iter_bpf_link__destroy(skel);
 }
 
+static void test_kallsyms_iter(void)
+{
+	struct bpf_iter_kallsyms *skel;
+
+	skel = bpf_iter_kallsyms__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "bpf_iter_kallsyms__open_and_load"))
+		return;
+
+	do_dummy_read(skel->progs.dump_kallsyms);
+
+	bpf_iter_kallsyms__destroy(skel);
+}
+
 #define CMP_BUFFER_SIZE 1024
 static char task_vma_output[CMP_BUFFER_SIZE];
 static char proc_maps_output[CMP_BUFFER_SIZE];
@@ -1267,4 +1281,6 @@ void test_bpf_iter(void)
 		test_buf_neg_offset();
 	if (test__start_subtest("link-iter"))
 		test_link_iter();
+	if (test__start_subtest("kallsyms"))
+		test_kallsyms_iter();
 }
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_kallsyms.c b/tools/testing/selftests/bpf/progs/bpf_iter_kallsyms.c
new file mode 100644
index 0000000..20c8d85
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_kallsyms.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022, Oracle and/or its affiliates. */
+#include "bpf_iter.h"
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+unsigned long last_sym_value = 0;
+
+static inline char tolower(char c)
+{
+	if (c >= 'A' && c <= 'Z')
+		c += ('a' - 'A');
+	return c;
+}
+
+static inline char toupper(char c)
+{
+	if (c >= 'a' && c <= 'z')
+		c -= ('a' - 'A');
+	return c;
+}
+
+/* Dump symbols with max size; the latter is calculated by caching symbol N value
+ * and when iterating on symbol N+1, we can print max size of symbol N via
+ * address of N+1 - address of N.
+ */
+SEC("iter/kallsyms")
+int dump_kallsyms(struct bpf_iter__kallsyms *ctx)
+{
+	struct seq_file *seq = ctx->meta->seq;
+	struct kallsym_iter *iter = ctx->kallsym_iter;
+	__u32 seq_num = ctx->meta->seq_num;
+	char type;
+	int ret;
+
+	if (!iter)
+		return 0;
+
+	if (seq_num == 0) {
+		BPF_SEQ_PRINTF(seq, "ADDR TYPE NAME MODULE_NAME KIND MAX_SIZE\n");
+		return 0;
+	}
+	if (last_sym_value)
+		BPF_SEQ_PRINTF(seq, "0x%x\n", iter->value - last_sym_value);
+	else
+		BPF_SEQ_PRINTF(seq, "\n");
+
+	last_sym_value = iter->value;
+
+	type = iter->type;
+
+	if (iter->module_name[0]) {
+		type = iter->exported ? toupper(type) : tolower(type);
+		BPF_SEQ_PRINTF(seq, "0x%llx %c %s [ %s ] ",
+			       iter->value, type, iter->name, iter->module_name);
+	} else {
+		BPF_SEQ_PRINTF(seq, "0x%llx %c %s ", iter->value, type, iter->name);
+	}
+	if (!iter->pos_arch_end || iter->pos_arch_end > iter->pos)
+		BPF_SEQ_PRINTF(seq, "CORE ");
+	else if (!iter->pos_mod_end || iter->pos_mod_end > iter->pos)
+		BPF_SEQ_PRINTF(seq, "MOD ");
+	else if (!iter->pos_ftrace_mod_end || iter->pos_ftrace_mod_end > iter->pos)
+		BPF_SEQ_PRINTF(seq, "FTRACE_MOD ");
+	else if (!iter->pos_bpf_end || iter->pos_bpf_end > iter->pos)
+		BPF_SEQ_PRINTF(seq, "BPF ");
+	else
+		BPF_SEQ_PRINTF(seq, "KPROBE ");
+	return 0;
+}
-- 
1.8.3.1

Re: [RFC bpf-next 1/2] bpf: add a kallsyms BPF iterator

[Andrii Nakryiko]

On Fri, Jun 24, 2022 at 9:45 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> add a "kallsyms" iterator which provides access to a "struct kallsym_iter"
> for each symbol.  Intent is to support more flexible symbol parsing
> as discussed in [1].
>
> [1] https://lore.kernel.org/all/YjRPZj6Z8vuLeEZo@krava/
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  kernel/kallsyms.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
>
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index fbdf8d3..ffaf464 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -30,6 +30,7 @@
>  #include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/bsearch.h>
> +#include <linux/btf_ids.h>
>
>  /*
>   * These will be re-linked against their real values
> @@ -799,6 +800,95 @@ static int s_show(struct seq_file *m, void *p)
>         .show = s_show
>  };
>
> +#ifdef CONFIG_BPF_SYSCALL
> +
> +struct bpf_iter__kallsyms {

So I know this is derived from /proc/kallsyms, but for BPF iterators
we have a singular name convention (e.g., iter/task and
iter/task_vma), which makes sense because we call BPF program for each
individual item. So here it seems like "iter/ksym" would make good
sense?

> +       __bpf_md_ptr(struct bpf_iter_meta *, meta);
> +       __bpf_md_ptr(struct kallsym_iter *, kallsym_iter);

nit: can we call this field just "ksym"?

> +};
> +
> +static int s_prog_seq_show(struct seq_file *m, bool in_stop)
> +{
> +       struct bpf_iter__kallsyms ctx;
> +       struct bpf_iter_meta meta;
> +       struct bpf_prog *prog;
> +
> +       meta.seq = m;
> +       prog = bpf_iter_get_info(&meta, in_stop);
> +       if (!prog)
> +               return 0;
> +
> +       ctx.meta = &meta;
> +       ctx.kallsym_iter = m ? m->private : NULL;
> +       return bpf_iter_run_prog(prog, &ctx);
> +}
> +
> +static int bpf_iter_s_seq_show(struct seq_file *m, void *p)

stupid question, but what does "_s_" part stand for? Is it for "sym"?
If yes, maybe then "bpf_iter_ksym_seq_show"?

> +{
> +       return s_prog_seq_show(m, false);
> +}
> +

[...]

> +static struct bpf_iter_reg kallsyms_iter_reg_info = {
> +       .target                 = "kallsyms",
> +       .ctx_arg_info_size      = 1,
> +       .ctx_arg_info           = {
> +               { offsetof(struct bpf_iter__kallsyms, kallsym_iter),
> +                 PTR_TO_BTF_ID_OR_NULL },
> +       },
> +       .seq_info               = &kallsyms_iter_seq_info,
> +};
> +
> +BTF_ID_LIST(btf_kallsym_iter_id)
> +BTF_ID(struct, kallsym_iter)
> +
> +static void __init bpf_kallsyms_iter_register(void)
> +{
> +       kallsyms_iter_reg_info.ctx_arg_info[0].btf_id = *btf_kallsym_iter_id;
> +       if (bpf_iter_reg_target(&kallsyms_iter_reg_info))
> +               pr_warn("Warning: could not register bpf kallsyms iterator\n");
> +}
> +
> +#endif /* CONFIG_PROC_FS */

Is there any reason to depend on CONFIG_PROC_FS for BPF iterator?
Seems like kernel/kallsyms.c itself is only depending on
CONFIG_KALLSYMS? So why adding unnecessary dependency?

> +
> +#endif /* CONFIG_BPF_SYSCALL */
> +
>  static inline int kallsyms_for_perf(void)
>  {
>  #ifdef CONFIG_PERF_EVENTS
> @@ -885,6 +975,9 @@ const char *kdb_walk_kallsyms(loff_t *pos)
>  static int __init kallsyms_init(void)
>  {
>         proc_create("kallsyms", 0444, NULL, &kallsyms_proc_ops);
> +#if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_PROC_FS)
> +       bpf_kallsyms_iter_register();
> +#endif
>         return 0;
>  }
>  device_initcall(kallsyms_init);
> --
> 1.8.3.1
>