All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v2] libbpf: support weak typed ksyms.
@ 2021-08-10 18:06 Hao Luo
  2021-08-11 21:47 ` Andrii Nakryiko
  0 siblings, 1 reply; 6+ messages in thread
From: Hao Luo @ 2021-08-10 18:06 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
if they can not be resolved. This means that if a bpf object contains
the declaration of a nonexistent weak typed ksym, it will be rejected
even if there is no program that references the symbol.

Nonexistent weak typed ksyms can also default to zero just like
typeless ones. This allows programs that access weak typed ksyms to be
accepted by verifier, if the accesses are guarded. For example,

extern const int bpf_link_fops3 __ksym __weak;

/* then in BPF program */

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

If actual use of nonexistent typed ksym 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.

Signed-off-by: Hao Luo <haoluo@google.com>
---
Changes since v1:
 - Weak typed symbols default to zero, as suggested by Andrii.
 - Use ASSERT_XXX() for tets.

 tools/lib/bpf/libbpf.c                        | 17 ++++--
 .../selftests/bpf/prog_tests/ksyms_btf.c      | 25 ++++++++
 .../selftests/bpf/progs/test_ksyms_weak.c     | 57 +++++++++++++++++++
 3 files changed, 94 insertions(+), 5 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..e7547a2967cf 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5277,11 +5277,11 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
 				}
 				insn[1].imm = ext->kcfg.data_off;
 			} else /* EXT_KSYM */ {
-				if (ext->ksym.type_id) { /* typed ksyms */
+				if (ext->ksym.type_id && ext->is_set) { /* typed ksyms */
 					insn[0].src_reg = BPF_PSEUDO_BTF_ID;
 					insn[0].imm = ext->ksym.kernel_btf_id;
 					insn[1].imm = ext->ksym.kernel_btf_obj_fd;
-				} else { /* typeless ksyms */
+				} else { /* typeless ksyms or unresolved typed ksyms */
 					insn[0].imm = (__u32)ext->ksym.addr;
 					insn[1].imm = ext->ksym.addr >> 32;
 				}
@@ -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,15 @@ 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)
+		return 0;
+
 	/* find local type_id */
 	local_type_id = ext->ksym.type_id;
 
@@ -6676,7 +6683,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..6bd60902f200 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,27 @@ 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, "test_ksyms_weak__open_and_load", "failed\n"))
+		return;
+
+	/* trigger tracepoint */
+	usleep(1);
+
+	data = skel->data;
+	ASSERT_EQ(data->out__existing_typed, 0, "existing typed ksym");
+	ASSERT_NEQ(data->out__existing_typeless, -1, "existing typeless ksym");
+	ASSERT_EQ(data->out__non_existent_typeless, 0, "nonexistent typeless ksym");
+	ASSERT_EQ(data->out__non_existent_typed, 0, "nonexistent typed ksym");
+
+	test_ksyms_weak__destroy(skel);
+}
+
 void test_ksyms_btf(void)
 {
 	int percpu_datasec;
@@ -105,4 +127,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..8a7d69ddd89a
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_ksyms_weak.c
@@ -0,0 +1,57 @@
+// 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_existent_typeless = -1;
+__u64 out__non_existent_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-existent weak symbols. */
+
+/* typeless symbols, default to zero. */
+extern const void bpf_link_fops1 __ksym __weak;
+
+/* typed symbols, default to zero. */
+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-existent symbols. */
+	out__non_existent_typeless = (__u64)&bpf_link_fops1;
+
+	/* tests non-existent symbols. */
+	out__non_existent_typed = (__u64)&bpf_link_fops2;
+
+	if (&bpf_link_fops2) /* can't happen */
+		out__non_existent_typed = (__u64)bpf_per_cpu_ptr(&bpf_link_fops2, 0);
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.32.0.605.g8dce9f2422-goog


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

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

On Tue, Aug 10, 2021 at 11:22 AM 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
> if they can not be resolved. This means that if a bpf object contains
> the declaration of a nonexistent weak typed ksym, it will be rejected
> even if there is no program that references the symbol.
>
> Nonexistent weak typed ksyms can also default to zero just like
> typeless ones. This allows programs that access weak typed ksyms to be
> accepted by verifier, if the accesses are guarded. For example,
>
> extern const int bpf_link_fops3 __ksym __weak;
>
> /* then in BPF program */
>
> if (&bpf_link_fops3) {
>    /* use bpf_link_fops3 */
> }
>
> If actual use of nonexistent typed ksym 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.
>
> Signed-off-by: Hao Luo <haoluo@google.com>
> ---

Selftests are failing ([0]). Did they work locally?..

  test_weak_syms:PASS:test_ksyms_weak__open_and_load 0 nsec
  test_weak_syms:FAIL:existing typed ksym unexpected existing typed
ksym: actual -1 != expected 0
  test_weak_syms:FAIL:existing typeless ksym unexpected existing
typeless ksym: actual -1 == expected -1
  test_weak_syms:FAIL:nonexistent typeless ksym unexpected nonexistent
typeless ksym: actual -1 != expected 0
  test_weak_syms:FAIL:nonexistent typed ksym unexpected nonexistent
typed ksym: actual -1 != expected 0
  #59/3 weak_ksyms:FAIL
  #59 ksyms_btf:FAIL


  [0] https://github.com/kernel-patches/bpf/pull/1611/checks?check_run_id=3305288074

> Changes since v1:
>  - Weak typed symbols default to zero, as suggested by Andrii.
>  - Use ASSERT_XXX() for tets.
>
>  tools/lib/bpf/libbpf.c                        | 17 ++++--
>  .../selftests/bpf/prog_tests/ksyms_btf.c      | 25 ++++++++
>  .../selftests/bpf/progs/test_ksyms_weak.c     | 57 +++++++++++++++++++
>  3 files changed, 94 insertions(+), 5 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_weak.c
>

[...]

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

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

On Wed, Aug 11, 2021 at 2:47 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Aug 10, 2021 at 11:22 AM 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
> > if they can not be resolved. This means that if a bpf object contains
> > the declaration of a nonexistent weak typed ksym, it will be rejected
> > even if there is no program that references the symbol.
> >
> > Nonexistent weak typed ksyms can also default to zero just like
> > typeless ones. This allows programs that access weak typed ksyms to be
> > accepted by verifier, if the accesses are guarded. For example,
> >
> > extern const int bpf_link_fops3 __ksym __weak;
> >
> > /* then in BPF program */
> >
> > if (&bpf_link_fops3) {
> >    /* use bpf_link_fops3 */
> > }
> >
> > If actual use of nonexistent typed ksym 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.
> >
> > Signed-off-by: Hao Luo <haoluo@google.com>
> > ---
>
> Selftests are failing ([0]). Did they work locally?..
>
>   test_weak_syms:PASS:test_ksyms_weak__open_and_load 0 nsec
>   test_weak_syms:FAIL:existing typed ksym unexpected existing typed
> ksym: actual -1 != expected 0
>   test_weak_syms:FAIL:existing typeless ksym unexpected existing
> typeless ksym: actual -1 == expected -1
>   test_weak_syms:FAIL:nonexistent typeless ksym unexpected nonexistent
> typeless ksym: actual -1 != expected 0
>   test_weak_syms:FAIL:nonexistent typed ksym unexpected nonexistent
> typed ksym: actual -1 != expected 0
>   #59/3 weak_ksyms:FAIL
>   #59 ksyms_btf:FAIL
>

Doh, in the test I accidentally removed the line of attachment in the
last minute of sending out... :) Let me resend quickly.


>
>   [0] https://github.com/kernel-patches/bpf/pull/1611/checks?check_run_id=3305288074
>
> > Changes since v1:
> >  - Weak typed symbols default to zero, as suggested by Andrii.
> >  - Use ASSERT_XXX() for tets.
> >
> >  tools/lib/bpf/libbpf.c                        | 17 ++++--
> >  .../selftests/bpf/prog_tests/ksyms_btf.c      | 25 ++++++++
> >  .../selftests/bpf/progs/test_ksyms_weak.c     | 57 +++++++++++++++++++
> >  3 files changed, 94 insertions(+), 5 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_weak.c
> >
>
> [...]

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

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

On Wed, Aug 11, 2021 at 4:41 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Aug 11, 2021 at 3:40 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
> > if they can not be resolved. This means that if a bpf object contains
> > the declaration of a nonexistent weak typed ksym, it will be rejected
> > even if there is no program that references the symbol.
> >
> > Nonexistent weak typed ksyms can also default to zero just like
> > typeless ones. This allows programs that access weak typed ksyms to be
> > accepted by verifier, if the accesses are guarded. For example,
> >
> > extern const int bpf_link_fops3 __ksym __weak;
> >
> > /* then in BPF program */
> >
> > if (&bpf_link_fops3) {
> >    /* use bpf_link_fops3 */
> > }
> >
> > If actual use of nonexistent typed ksym 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.
> >
> > Signed-off-by: Hao Luo <haoluo@google.com>
> > ---
Thanks for taking a look! All the comments seem reasonable. Let me fix
them, test and send a new version.

> >  Changes since v1:
> >   - Weak typed symbols default to zero, as suggested by Andrii.
> >   - Use ASSERT_XXX() for tests.
> >
> >  tools/lib/bpf/libbpf.c                        | 17 ++++--
> >  .../selftests/bpf/prog_tests/ksyms_btf.c      | 31 ++++++++++
> >  .../selftests/bpf/progs/test_ksyms_weak.c     | 57 +++++++++++++++++++
> >  3 files changed, 100 insertions(+), 5 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_weak.c
> >
[...]

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

* Re: [PATCH bpf-next v2] libbpf: support weak typed ksyms.
  2021-08-11 22:40 Hao Luo
@ 2021-08-11 23:41 ` Andrii Nakryiko
  2021-08-12  0:20   ` Hao Luo
  0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2021-08-11 23:41 UTC (permalink / raw)
  To: Hao Luo
  Cc: Networking, bpf, Alexei Starovoitov, Andrii Nakryiko,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song

On Wed, Aug 11, 2021 at 3:40 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
> if they can not be resolved. This means that if a bpf object contains
> the declaration of a nonexistent weak typed ksym, it will be rejected
> even if there is no program that references the symbol.
>
> Nonexistent weak typed ksyms can also default to zero just like
> typeless ones. This allows programs that access weak typed ksyms to be
> accepted by verifier, if the accesses are guarded. For example,
>
> extern const int bpf_link_fops3 __ksym __weak;
>
> /* then in BPF program */
>
> if (&bpf_link_fops3) {
>    /* use bpf_link_fops3 */
> }
>
> If actual use of nonexistent typed ksym 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.
>
> Signed-off-by: Hao Luo <haoluo@google.com>
> ---
>  Changes since v1:
>   - Weak typed symbols default to zero, as suggested by Andrii.
>   - Use ASSERT_XXX() for tests.
>
>  tools/lib/bpf/libbpf.c                        | 17 ++++--
>  .../selftests/bpf/prog_tests/ksyms_btf.c      | 31 ++++++++++
>  .../selftests/bpf/progs/test_ksyms_weak.c     | 57 +++++++++++++++++++
>  3 files changed, 100 insertions(+), 5 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..e7547a2967cf 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -5277,11 +5277,11 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
>                                 }
>                                 insn[1].imm = ext->kcfg.data_off;
>                         } else /* EXT_KSYM */ {
> -                               if (ext->ksym.type_id) { /* typed ksyms */
> +                               if (ext->ksym.type_id && ext->is_set) { /* typed ksyms */
>                                         insn[0].src_reg = BPF_PSEUDO_BTF_ID;
>                                         insn[0].imm = ext->ksym.kernel_btf_id;
>                                         insn[1].imm = ext->ksym.kernel_btf_obj_fd;
> -                               } else { /* typeless ksyms */
> +                               } else { /* typeless ksyms or unresolved typed ksyms */
>                                         insn[0].imm = (__u32)ext->ksym.addr;
>                                         insn[1].imm = ext->ksym.addr >> 32;
>                                 }
> @@ -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,

instead of teaching find_ksym_btf_id() about weak symbol, just handle
-ESRCH in bpf_object__resolve_ksym_var_btf_id (you already are
special-handling it anyway). Just move the pr_warn from
find_ksym_btf_id into bpf_object__resolve_ksym_var_btf_id.
bpf_object__resolve_ksym_func_btf_id() already has a relevant pr_warn,
which duplicates the one in find_ksym_btf_id.

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

so this is not needed

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

and this will be moved out to not emit unnecessary warnings for weak symbols

> @@ -6627,11 +6630,15 @@ 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)
> +               return 0;
> +

and this will handle ESRCH + is_weak as a special case

>         /* find local type_id */
>         local_type_id = ext->ksym.type_id;
>
> @@ -6676,7 +6683,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",

[...]

> +/* 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-existent weak symbols. */
> +
> +/* typeless symbols, default to zero. */
> +extern const void bpf_link_fops1 __ksym __weak;
> +
> +/* typed symbols, default to zero. */
> +extern const int bpf_link_fops2 __ksym __weak;
> +
> +/* typed symbols, pass if not referenced. */
> +extern const int bpf_link_fops3 __ksym __weak;
> +

this will be compiled out by compiler, you are not really testing
anything with that (libbpf doesn't even know about bpf_link_fops3).
Just drop bpf_link_fops3, bpf_link_fops2 is testing everything anyway.

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

empty line between variable declaration and statements (better declare
and initialize rq separately, with empty line between those two)

> +       if (rq)
> +               out__existing_typed = rq->cpu;
> +       out__existing_typeless = (__u64)&bpf_prog_active;
> +
> +       /* tests non-existent symbols. */
> +       out__non_existent_typeless = (__u64)&bpf_link_fops1;
> +
> +       /* tests non-existent symbols. */
> +       out__non_existent_typed = (__u64)&bpf_link_fops2;
> +
> +       if (&bpf_link_fops2) /* can't happen */
> +               out__non_existent_typed = (__u64)bpf_per_cpu_ptr(&bpf_link_fops2, 0);
> +
> +       return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.32.0.605.g8dce9f2422-goog
>

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

* [PATCH bpf-next v2] libbpf: support weak typed ksyms.
@ 2021-08-11 22:40 Hao Luo
  2021-08-11 23:41 ` Andrii Nakryiko
  0 siblings, 1 reply; 6+ messages in thread
From: Hao Luo @ 2021-08-11 22:40 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
if they can not be resolved. This means that if a bpf object contains
the declaration of a nonexistent weak typed ksym, it will be rejected
even if there is no program that references the symbol.

Nonexistent weak typed ksyms can also default to zero just like
typeless ones. This allows programs that access weak typed ksyms to be
accepted by verifier, if the accesses are guarded. For example,

extern const int bpf_link_fops3 __ksym __weak;

/* then in BPF program */

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

If actual use of nonexistent typed ksym 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.

Signed-off-by: Hao Luo <haoluo@google.com>
---
 Changes since v1:
  - Weak typed symbols default to zero, as suggested by Andrii.
  - Use ASSERT_XXX() for tests.

 tools/lib/bpf/libbpf.c                        | 17 ++++--
 .../selftests/bpf/prog_tests/ksyms_btf.c      | 31 ++++++++++
 .../selftests/bpf/progs/test_ksyms_weak.c     | 57 +++++++++++++++++++
 3 files changed, 100 insertions(+), 5 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..e7547a2967cf 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5277,11 +5277,11 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
 				}
 				insn[1].imm = ext->kcfg.data_off;
 			} else /* EXT_KSYM */ {
-				if (ext->ksym.type_id) { /* typed ksyms */
+				if (ext->ksym.type_id && ext->is_set) { /* typed ksyms */
 					insn[0].src_reg = BPF_PSEUDO_BTF_ID;
 					insn[0].imm = ext->ksym.kernel_btf_id;
 					insn[1].imm = ext->ksym.kernel_btf_obj_fd;
-				} else { /* typeless ksyms */
+				} else { /* typeless ksyms or unresolved typed ksyms */
 					insn[0].imm = (__u32)ext->ksym.addr;
 					insn[1].imm = ext->ksym.addr >> 32;
 				}
@@ -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,15 @@ 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)
+		return 0;
+
 	/* find local type_id */
 	local_type_id = ext->ksym.type_id;
 
@@ -6676,7 +6683,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..cf3acfa5a91d 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,33 @@ 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;
+	int err;
+
+	skel = test_ksyms_weak__open_and_load();
+	if (CHECK(!skel, "test_ksyms_weak__open_and_load", "failed\n"))
+		return;
+
+	err = test_ksyms_weak__attach(skel);
+	if (CHECK(err, "test_ksyms_weak__attach", "skeleton attach failed: %d\n", err))
+		goto cleanup;
+
+	/* trigger tracepoint */
+	usleep(1);
+
+	data = skel->data;
+	ASSERT_EQ(data->out__existing_typed, 0, "existing typed ksym");
+	ASSERT_NEQ(data->out__existing_typeless, -1, "existing typeless ksym");
+	ASSERT_EQ(data->out__non_existent_typeless, 0, "nonexistent typeless ksym");
+	ASSERT_EQ(data->out__non_existent_typed, 0, "nonexistent typed ksym");
+
+cleanup:
+	test_ksyms_weak__destroy(skel);
+}
+
 void test_ksyms_btf(void)
 {
 	int percpu_datasec;
@@ -105,4 +133,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..9b099fa59822
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_ksyms_weak.c
@@ -0,0 +1,57 @@
+// 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_existent_typeless = -1;
+__u64 out__non_existent_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-existent weak symbols. */
+
+/* typeless symbols, default to zero. */
+extern const void bpf_link_fops1 __ksym __weak;
+
+/* typed symbols, default to zero. */
+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-existent symbols. */
+	out__non_existent_typeless = (__u64)&bpf_link_fops1;
+
+	/* tests non-existent symbols. */
+	out__non_existent_typed = (__u64)&bpf_link_fops2;
+
+	if (&bpf_link_fops2) /* can't happen */
+		out__non_existent_typed = (__u64)bpf_per_cpu_ptr(&bpf_link_fops2, 0);
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.32.0.605.g8dce9f2422-goog


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

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

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 18:06 [PATCH bpf-next v2] libbpf: support weak typed ksyms Hao Luo
2021-08-11 21:47 ` Andrii Nakryiko
2021-08-11 22:13   ` Hao Luo
2021-08-11 22:40 Hao Luo
2021-08-11 23:41 ` Andrii Nakryiko
2021-08-12  0:20   ` Hao Luo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.