Message-Id: <20210802212815.3488773-1-haoluo@google.com>
Date:   Mon,  2 Aug 2021 14:28:15 -0700
From:   Hao Luo <haoluo@...gle.com>
To:     netdev@...r.kernel.org, bpf@...r.kernel.org
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andriin@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Hao Luo <haoluo@...gle.com>
Subject: [PATCH bpf-next] libbpf: support weak typed ksyms.

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@...gle.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

Powered by blists - more mailing lists