Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH bpf-next] libbpf: support weak typed ksyms.
@ 2021-08-02 21:28 Hao Luo
  2021-08-06 22:40 ` Andrii Nakryiko
  0 siblings, 1 reply; 3+ messages in thread
From: Hao Luo @ 2021-08-02 21:28 UTC (permalink / raw)
  To: netdev, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, Hao Luo

Currently weak typeless ksyms have default value zero, when they don't
exist in the kernel. However, weak typed ksyms are rejected by libbpf.
This means that if a bpf object contains the declaration of a
non-existing weak typed ksym, it will be rejected even if there is
no program that references the symbol.

In fact, we could let them to pass the checks in libbpf and leave the
object to be rejected by the bpf verifier. More specifically, upon
seeing a weak typed symbol, libbpf can assign it a zero btf_id, which
is associated to the type 'void'. The verifier expects the symbol to
be BTF_VAR_KIND instead, therefore will reject loading.

In practice, we often add new kernel symbols and roll out the kernel
changes to fleet. And we want to release a single bpf object that can
be loaded on both the new and the old kernels. Passing weak typed ksyms
in libbpf allows us to do so as long as the programs that reference the
new symbols are disabled on the old kernel.

Signed-off-by: Hao Luo <haoluo@google.com>
---
 tools/lib/bpf/libbpf.c                        | 17 +++++-
 .../selftests/bpf/prog_tests/ksyms_btf.c      | 42 +++++++++++++
 .../selftests/bpf/progs/test_ksyms_weak.c     | 60 +++++++++++++++++++
 3 files changed, 116 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_weak.c

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index cb106e8c42cb..1cac02bfa599 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6584,7 +6584,7 @@ static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
 }
 
 static int find_ksym_btf_id(struct bpf_object *obj, const char *ksym_name,
-			    __u16 kind, struct btf **res_btf,
+			    __u16 kind, bool is_weak, struct btf **res_btf,
 			    int *res_btf_fd)
 {
 	int i, id, btf_fd, err;
@@ -6608,6 +6608,9 @@ static int find_ksym_btf_id(struct bpf_object *obj, const char *ksym_name,
 				break;
 		}
 	}
+	if (is_weak && id == -ENOENT)
+		return 0;
+
 	if (id <= 0) {
 		pr_warn("extern (%s ksym) '%s': failed to find BTF ID in kernel BTF(s).\n",
 			__btf_kind_str(kind), ksym_name);
@@ -6627,11 +6630,19 @@ static int bpf_object__resolve_ksym_var_btf_id(struct bpf_object *obj,
 	const char *targ_var_name;
 	int id, btf_fd = 0, err;
 	struct btf *btf = NULL;
+	bool is_weak = ext->is_weak;
 
-	id = find_ksym_btf_id(obj, ext->name, BTF_KIND_VAR, &btf, &btf_fd);
+	id = find_ksym_btf_id(obj, ext->name, BTF_KIND_VAR, is_weak, &btf, &btf_fd);
 	if (id < 0)
 		return id;
 
+	if (is_weak && id == 0) {
+		ext->is_set = true;
+		ext->ksym.kernel_btf_obj_fd = 0;
+		ext->ksym.kernel_btf_id = 0;
+		return 0;
+	}
+
 	/* find local type_id */
 	local_type_id = ext->ksym.type_id;
 
@@ -6676,7 +6687,7 @@ static int bpf_object__resolve_ksym_func_btf_id(struct bpf_object *obj,
 
 	local_func_proto_id = ext->ksym.type_id;
 
-	kfunc_id = find_ksym_btf_id(obj, ext->name, BTF_KIND_FUNC,
+	kfunc_id = find_ksym_btf_id(obj, ext->name, BTF_KIND_FUNC, false,
 				    &kern_btf, &kern_btf_fd);
 	if (kfunc_id < 0) {
 		pr_warn("extern (func ksym) '%s': not found in kernel BTF\n",
diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c b/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c
index 67bebd324147..12a5e5ebcc3d 100644
--- a/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c
@@ -6,6 +6,7 @@
 #include <bpf/btf.h>
 #include "test_ksyms_btf.skel.h"
 #include "test_ksyms_btf_null_check.skel.h"
+#include "test_ksyms_weak.skel.h"
 
 static int duration;
 
@@ -81,6 +82,44 @@ static void test_null_check(void)
 	test_ksyms_btf_null_check__destroy(skel);
 }
 
+static void test_weak_syms(void)
+{
+	struct test_ksyms_weak *skel;
+	struct test_ksyms_weak__data *data;
+
+	skel = test_ksyms_weak__open_and_load();
+	if (CHECK(skel, "skel_open_and_load",
+		  "unexpected load of a prog referencing non-existing ksyms\n")) {
+		test_ksyms_weak__destroy(skel);
+		return;
+	}
+
+	skel = test_ksyms_weak__open();
+	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
+		return;
+
+	bpf_program__set_autoload(skel->progs.fail_handler, false);
+	if (CHECK(test_ksyms_weak__load(skel), "skel_load", "failed to load skeleton\n"))
+		goto cleanup;
+
+	if (CHECK(test_ksyms_weak__attach(skel), "skel_attach", "failed to attach skeleton\n"))
+		goto cleanup;
+
+	/* trigger tracepoint */
+	usleep(1);
+
+	data = skel->data;
+	CHECK(data->out__existing_typed != 0, "existing_typed",
+	      "failed to reference an existing typed symbol\n");
+	CHECK(data->out__existing_typeless == -1, "existing_typeless",
+	      "failed to get the address of an existing typeless symbol\n");
+	CHECK(data->out__non_existing_typeless != 0, "non_existing_typeless",
+	      "non-existing typeless symbol does not default to zero\n");
+
+cleanup:
+	test_ksyms_weak__destroy(skel);
+}
+
 void test_ksyms_btf(void)
 {
 	int percpu_datasec;
@@ -105,4 +144,7 @@ void test_ksyms_btf(void)
 
 	if (test__start_subtest("null_check"))
 		test_null_check();
+
+	if (test__start_subtest("weak_ksyms"))
+		test_weak_syms();
 }
diff --git a/tools/testing/selftests/bpf/progs/test_ksyms_weak.c b/tools/testing/selftests/bpf/progs/test_ksyms_weak.c
new file mode 100644
index 000000000000..e956fdf3162c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_ksyms_weak.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test weak ksyms.
+ *
+ * Copyright (c) 2021 Google
+ */
+
+#include "vmlinux.h"
+
+#include <bpf/bpf_helpers.h>
+
+int out__existing_typed = -1;
+__u64 out__existing_typeless = -1;
+
+__u64 out__non_existing_typeless = -1;
+__u64 out__non_existing_typed = -1;
+
+/* existing weak symbols */
+
+/* test existing weak symbols can be resolved. */
+extern const struct rq runqueues __ksym __weak; /* typed */
+extern const void bpf_prog_active __ksym __weak; /* typeless */
+
+
+/* non-existing weak symbols. */
+
+/* typeless symbols, default to zero. */
+extern const void bpf_link_fops1 __ksym __weak;
+
+/* typed symbols, fail verifier checks if referenced. */
+extern const int bpf_link_fops2 __ksym __weak;
+
+/* typed symbols, pass if not referenced. */
+extern const int bpf_link_fops3 __ksym __weak;
+
+SEC("raw_tp/sys_enter")
+int pass_handler(const void *ctx)
+{
+	/* tests existing symbols. */
+	struct rq *rq = (struct rq *)bpf_per_cpu_ptr(&runqueues, 0);
+	if (rq)
+		out__existing_typed = rq->cpu;
+	out__existing_typeless = (__u64)&bpf_prog_active;
+
+	/* tests non-existing symbols. */
+	out__non_existing_typeless = (__u64)&bpf_link_fops1;
+
+	return 0;
+}
+
+SEC("raw_tp/sys_exit")
+int fail_handler(const void *ctx)
+{
+	/* tests non-existing symbols. */
+	out__non_existing_typed = (__u64)&bpf_link_fops2;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.32.0.554.ge1b32706d8-goog


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

* Re: [PATCH bpf-next] libbpf: support weak typed ksyms.
  2021-08-02 21:28 [PATCH bpf-next] libbpf: support weak typed ksyms Hao Luo
@ 2021-08-06 22:40 ` Andrii Nakryiko
  2021-08-07  0:48   ` Hao Luo
  0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2021-08-06 22:40 UTC (permalink / raw)
  To: Hao Luo
  Cc: Networking, bpf, Alexei Starovoitov, Andrii Nakryiko,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song

On Mon, Aug 2, 2021 at 2:29 PM Hao Luo <haoluo@google.com> wrote:
>
> Currently weak typeless ksyms have default value zero, when they don't
> exist in the kernel. However, weak typed ksyms are rejected by libbpf.
> This means that if a bpf object contains the declaration of a
> non-existing weak typed ksym, it will be rejected even if there is
> no program that references the symbol.
>
> In fact, we could let them to pass the checks in libbpf and leave the
> object to be rejected by the bpf verifier. More specifically, upon
> seeing a weak typed symbol, libbpf can assign it a zero btf_id, which
> is associated to the type 'void'. The verifier expects the symbol to
> be BTF_VAR_KIND instead, therefore will reject loading.
>
> In practice, we often add new kernel symbols and roll out the kernel
> changes to fleet. And we want to release a single bpf object that can
> be loaded on both the new and the old kernels. Passing weak typed ksyms
> in libbpf allows us to do so as long as the programs that reference the
> new symbols are disabled on the old kernel.

How do you detect whether a given ksym is present or not? You check
that from user-space and then use .rodata to turn off pieces of BPF
logic? That's quite inconvenient. It would be great if these typed
ksyms worked the same way as typeless ones:

extern const int bpf_link_fops3 __ksym __weak;

/* then in BPF program */

if (&bpf_link_fops3) {
   /* use bpf_link_fops3 */
}


I haven't tried, but I suspect it could be made to work if libbpf
replaces corresponding ldimm64 instruction (with BTF ID) into a plain
ldimm64 instruction loading 0 directly. That would allow the above
check (and it would be known false to the verifier) to succeed without
the verifier rejecting the BPF program. If actual use of non-existing
typed symbol is not guarded properly, verifier would see that register
is not PTR_TO_BTF_ID and wouldn't allow to use it for direct memory
reads or passing it to BPF helpers.

Have you considered such an approach?


Separately, please use ASSERT_XXX() macros for tests, not plain
CHECK()s. Thanks.

>
> Signed-off-by: Hao Luo <haoluo@google.com>
> ---
>  tools/lib/bpf/libbpf.c                        | 17 +++++-
>  .../selftests/bpf/prog_tests/ksyms_btf.c      | 42 +++++++++++++
>  .../selftests/bpf/progs/test_ksyms_weak.c     | 60 +++++++++++++++++++
>  3 files changed, 116 insertions(+), 3 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_weak.c
>

[...]

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

* Re: [PATCH bpf-next] libbpf: support weak typed ksyms.
  2021-08-06 22:40 ` Andrii Nakryiko
@ 2021-08-07  0:48   ` Hao Luo
  0 siblings, 0 replies; 3+ messages in thread
From: Hao Luo @ 2021-08-07  0:48 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Networking, bpf, Alexei Starovoitov, Andrii Nakryiko,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song

Thanks for taking a look.

On Fri, Aug 6, 2021 at 3:40 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Aug 2, 2021 at 2:29 PM Hao Luo <haoluo@google.com> wrote:
> >
> > Currently weak typeless ksyms have default value zero, when they don't
> > exist in the kernel. However, weak typed ksyms are rejected by libbpf.
> > This means that if a bpf object contains the declaration of a
> > non-existing weak typed ksym, it will be rejected even if there is
> > no program that references the symbol.
> >
> > In fact, we could let them to pass the checks in libbpf and leave the
> > object to be rejected by the bpf verifier. More specifically, upon
> > seeing a weak typed symbol, libbpf can assign it a zero btf_id, which
> > is associated to the type 'void'. The verifier expects the symbol to
> > be BTF_VAR_KIND instead, therefore will reject loading.
> >
> > In practice, we often add new kernel symbols and roll out the kernel
> > changes to fleet. And we want to release a single bpf object that can
> > be loaded on both the new and the old kernels. Passing weak typed ksyms
> > in libbpf allows us to do so as long as the programs that reference the
> > new symbols are disabled on the old kernel.
>
> How do you detect whether a given ksym is present or not? You check
> that from user-space and then use .rodata to turn off pieces of BPF
> logic? That's quite inconvenient. It would be great if these typed
> ksyms worked the same way as typeless ones:
>
It's not by detect. In my use case, I can add a flag to the
application to disable/enable loading a BPF program. Because we know
at which kernel version a new symbol was introduced, we can coordinate
the application flag with the kernel version to avoid the faulting
code being loaded on an old kernel.

> extern const int bpf_link_fops3 __ksym __weak;
>
> /* then in BPF program */
>
> if (&bpf_link_fops3) {
>    /* use bpf_link_fops3 */
> }
>
>
> I haven't tried, but I suspect it could be made to work if libbpf
> replaces corresponding ldimm64 instruction (with BTF ID) into a plain
> ldimm64 instruction loading 0 directly. That would allow the above
> check (and it would be known false to the verifier) to succeed without
> the verifier rejecting the BPF program. If actual use of non-existing
> typed symbol is not guarded properly, verifier would see that register
> is not PTR_TO_BTF_ID and wouldn't allow to use it for direct memory
> reads or passing it to BPF helpers.
>
> Have you considered such an approach?
>
I haven't thought about this approach. I just grabbed the quickest
solution I can think of. Will follow your suggestion and see if it
works.

>
> Separately, please use ASSERT_XXX() macros for tests, not plain
> CHECK()s. Thanks.
>
ACK.

> >
> > Signed-off-by: Hao Luo <haoluo@google.com>
> > ---
> >  tools/lib/bpf/libbpf.c                        | 17 +++++-
> >  .../selftests/bpf/prog_tests/ksyms_btf.c      | 42 +++++++++++++
> >  .../selftests/bpf/progs/test_ksyms_weak.c     | 60 +++++++++++++++++++
> >  3 files changed, 116 insertions(+), 3 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_weak.c
> >
>
> [...]

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

end of thread, other threads:[~2021-08-07  0:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 21:28 [PATCH bpf-next] libbpf: support weak typed ksyms Hao Luo
2021-08-06 22:40 ` Andrii Nakryiko
2021-08-07  0:48   ` Hao Luo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).