From 8568c3cefd5143fa0dc09f90e1bc9dc8905292f4 Mon Sep 17 00:00:00 2001 From: Martin KaFai Lau Date: Mon, 19 Oct 2020 12:42:25 -0700 Subject: [PATCH] bpf: selftest: Ensure the return value of the bpf_per_cpu_ptr() must be checked This patch tests all pointers returned by bpf_per_cpu_ptr() must be tested for NULL first before it can be accessed. This patch adds a subtest "null_check", so it moves the ".data..percpu" existence check to the very beginning and before doing any subtest. Signed-off-by: Martin KaFai Lau Signed-off-by: Alexei Starovoitov Link: https://lore.kernel.org/bpf/20201019194225.1051596-1-kafai@fb.com --- tools/testing/selftests/bpf/prog_tests/ksyms_btf.c | 57 +++++++++++++++------- .../bpf/progs/test_ksyms_btf_null_check.c | 31 ++++++++++++ 2 files changed, 70 insertions(+), 18 deletions(-) create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_btf_null_check.c diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c b/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c index 28e26bd3..b58b775 100644 --- a/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c +++ b/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c @@ -5,18 +5,17 @@ #include #include #include "test_ksyms_btf.skel.h" +#include "test_ksyms_btf_null_check.skel.h" static int duration; -void test_ksyms_btf(void) +static void test_basic(void) { __u64 runqueues_addr, bpf_prog_active_addr; __u32 this_rq_cpu; int this_bpf_prog_active; struct test_ksyms_btf *skel = NULL; struct test_ksyms_btf__data *data; - struct btf *btf; - int percpu_datasec; int err; err = kallsyms_find("runqueues", &runqueues_addr); @@ -31,20 +30,6 @@ void test_ksyms_btf(void) if (CHECK(err == -ENOENT, "ksym_find", "symbol 'bpf_prog_active' not found\n")) return; - btf = libbpf_find_kernel_btf(); - if (CHECK(IS_ERR(btf), "btf_exists", "failed to load kernel BTF: %ld\n", - PTR_ERR(btf))) - return; - - percpu_datasec = btf__find_by_name_kind(btf, ".data..percpu", - BTF_KIND_DATASEC); - if (percpu_datasec < 0) { - printf("%s:SKIP:no PERCPU DATASEC in kernel btf\n", - __func__); - test__skip(); - goto cleanup; - } - skel = test_ksyms_btf__open_and_load(); if (CHECK(!skel, "skel_open", "failed to open and load skeleton\n")) goto cleanup; @@ -83,6 +68,42 @@ void test_ksyms_btf(void) data->out__bpf_prog_active); cleanup: - btf__free(btf); test_ksyms_btf__destroy(skel); } + +static void test_null_check(void) +{ + struct test_ksyms_btf_null_check *skel; + + skel = test_ksyms_btf_null_check__open_and_load(); + CHECK(skel, "skel_open", "unexpected load of a prog missing null check\n"); + + test_ksyms_btf_null_check__destroy(skel); +} + +void test_ksyms_btf(void) +{ + int percpu_datasec; + struct btf *btf; + + btf = libbpf_find_kernel_btf(); + if (CHECK(IS_ERR(btf), "btf_exists", "failed to load kernel BTF: %ld\n", + PTR_ERR(btf))) + return; + + percpu_datasec = btf__find_by_name_kind(btf, ".data..percpu", + BTF_KIND_DATASEC); + btf__free(btf); + if (percpu_datasec < 0) { + printf("%s:SKIP:no PERCPU DATASEC in kernel btf\n", + __func__); + test__skip(); + return; + } + + if (test__start_subtest("basic")) + test_basic(); + + if (test__start_subtest("null_check")) + test_null_check(); +} diff --git a/tools/testing/selftests/bpf/progs/test_ksyms_btf_null_check.c b/tools/testing/selftests/bpf/progs/test_ksyms_btf_null_check.c new file mode 100644 index 0000000..8bc8f7c --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_ksyms_btf_null_check.c @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2020 Facebook */ + +#include "vmlinux.h" + +#include + +extern const struct rq runqueues __ksym; /* struct type global var. */ +extern const int bpf_prog_active __ksym; /* int type global var. */ + +SEC("raw_tp/sys_enter") +int handler(const void *ctx) +{ + struct rq *rq; + int *active; + __u32 cpu; + + cpu = bpf_get_smp_processor_id(); + rq = (struct rq *)bpf_per_cpu_ptr(&runqueues, cpu); + active = (int *)bpf_per_cpu_ptr(&bpf_prog_active, cpu); + if (active) { + /* READ_ONCE */ + *(volatile int *)active; + /* !rq has not been tested, so verifier should reject. */ + *(volatile int *)(&rq->cpu); + } + + return 0; +} + +char _license[] SEC("license") = "GPL"; -- 2.7.4