| Message ID | 20210802212815.3488773-1-haoluo@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] libbpf: support weak typed ksyms. | expand |
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 > [...]
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 > > > > [...]
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";
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