On Mon, Mar 11, 2019 at 2:51 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> From: Joe Stringer <joe@xxxxxxxxxxx>
>
> Add tests for libbpf relocation of static variable references
> into the .data, .rodata and .bss sections of the ELF, also add
> read-only test for .rodata. All passing:
>
> # ./test_progs
> [...]
> test_global_data:PASS:load program 0 nsec
> test_global_data:PASS:pass global data run 925 nsec
> test_global_data_number:PASS:relocate .bss reference 925 nsec
> test_global_data_number:PASS:relocate .data reference 925 nsec
> test_global_data_number:PASS:relocate .rodata reference 925 nsec
> test_global_data_number:PASS:relocate .bss reference 925 nsec
> test_global_data_number:PASS:relocate .data reference 925 nsec
> test_global_data_number:PASS:relocate .rodata reference 925 nsec
> test_global_data_number:PASS:relocate .bss reference 925 nsec
> test_global_data_number:PASS:relocate .bss reference 925 nsec
> test_global_data_number:PASS:relocate .rodata reference 925 nsec
> test_global_data_number:PASS:relocate .rodata reference 925 nsec
> test_global_data_number:PASS:relocate .rodata reference 925 nsec
> test_global_data_string:PASS:relocate .rodata reference 925 nsec
> test_global_data_string:PASS:relocate .data reference 925 nsec
> test_global_data_string:PASS:relocate .bss reference 925 nsec
> test_global_data_string:PASS:relocate .data reference 925 nsec
> test_global_data_string:PASS:relocate .bss reference 925 nsec
> test_global_data_struct:PASS:relocate .rodata reference 925 nsec
> test_global_data_struct:PASS:relocate .bss reference 925 nsec
> test_global_data_struct:PASS:relocate .rodata reference 925 nsec
> test_global_data_struct:PASS:relocate .data reference 925 nsec
> test_global_data_rdonly:PASS:test .rodata read-only map 925 nsec
> [...]
> Summary: 229 PASSED, 0 FAILED
>
> Note map helper signatures have been changed to avoid warnings
> when passing in const data.
>
> Joint work with Daniel Borkmann.
>
> Signed-off-by: Joe Stringer <joe@xxxxxxxxxxx>
> Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> ---
Looks good!
Just wanted to mention, that there is one interesting and useful case,
which isn't yet supported:
static const char *varstr = "This string will go into one of
.rodata.str1.* section";
It could be supported today with libbpf doing corresponding
relocations, but beyond that there is no good zero-terminated strings
support yet, so I think it's ok to postpone this. And from the
program's perspective, .data variables will have pointers into .rodata
map, which will need to be handled explicitly today, which is also
quite not nice.
Acked-by: Andrii Nakryiko <andriin@xxxxxx>
> tools/testing/selftests/bpf/bpf_helpers.h | 8 +-
> .../selftests/bpf/prog_tests/global_data.c | 157 ++++++++++++++++++
> .../selftests/bpf/progs/test_global_data.c | 106 ++++++++++++
> 3 files changed, 267 insertions(+), 4 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/global_data.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_data.c
>
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index c9433a496d54..91c53dac95c8 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -9,14 +9,14 @@
> #define SEC(NAME) __attribute__((section(NAME), used))
>
> /* helper functions called from eBPF programs written in C */
> -static void *(*bpf_map_lookup_elem)(void *map, void *key) =
> +static void *(*bpf_map_lookup_elem)(void *map, const void *key) =
> (void *) BPF_FUNC_map_lookup_elem;
> -static int (*bpf_map_update_elem)(void *map, void *key, void *value,
> +static int (*bpf_map_update_elem)(void *map, const void *key, const void *value,
> unsigned long long flags) =
> (void *) BPF_FUNC_map_update_elem;
> -static int (*bpf_map_delete_elem)(void *map, void *key) =
> +static int (*bpf_map_delete_elem)(void *map, const void *key) =
> (void *) BPF_FUNC_map_delete_elem;
> -static int (*bpf_map_push_elem)(void *map, void *value,
> +static int (*bpf_map_push_elem)(void *map, const void *value,
> unsigned long long flags) =
> (void *) BPF_FUNC_map_push_elem;
> static int (*bpf_map_pop_elem)(void *map, void *value) =
> diff --git a/tools/testing/selftests/bpf/prog_tests/global_data.c b/tools/testing/selftests/bpf/prog_tests/global_data.c
> new file mode 100644
> index 000000000000..d011079fb0bf
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/global_data.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +
> +static void test_global_data_number(struct bpf_object *obj, __u32 duration)
> +{
> + int i, err, map_fd;
> + uint64_t num;
> +
> + map_fd = bpf_find_map(__func__, obj, "result_number");
> + if (map_fd < 0) {
> + error_cnt++;
> + return;
> + }
> +
> + struct {
> + char *name;
> + uint32_t key;
> + uint64_t num;
> + } tests[] = {
> + { "relocate .bss reference", 0, 0 },
> + { "relocate .data reference", 1, 42 },
> + { "relocate .rodata reference", 2, 24 },
> + { "relocate .bss reference", 3, 0 },
> + { "relocate .data reference", 4, 0xffeeff },
> + { "relocate .rodata reference", 5, 0xabab },
> + { "relocate .bss reference", 6, 1234 },
> + { "relocate .bss reference", 7, 0 },
> + { "relocate .rodata reference", 8, 0xab },
> + { "relocate .rodata reference", 9, 0x1111111111111111 },
> + { "relocate .rodata reference", 10, ~0 },
> + };
> +
> + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
> + err = bpf_map_lookup_elem(map_fd, &tests[i].key, &num);
> + CHECK(err || num != tests[i].num, tests[i].name,
> + "err %d result %lx expected %lx\n",
> + err, num, tests[i].num);
> + }
> +}
> +
> +static void test_global_data_string(struct bpf_object *obj, __u32 duration)
> +{
> + int i, err, map_fd;
> + char str[32];
> +
> + map_fd = bpf_find_map(__func__, obj, "result_string");
> + if (map_fd < 0) {
> + error_cnt++;
> + return;
> + }
> +
> + struct {
> + char *name;
> + uint32_t key;
> + char str[32];
> + } tests[] = {
> + { "relocate .rodata reference", 0, "abcdefghijklmnopqrstuvwxyz" },
> + { "relocate .data reference", 1, "abcdefghijklmnopqrstuvwxyz" },
> + { "relocate .bss reference", 2, "" },
> + { "relocate .data reference", 3, "abcdexghijklmnopqrstuvwxyz" },
> + { "relocate .bss reference", 4, "\0\0hello" },
> + };
> +
> + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
> + err = bpf_map_lookup_elem(map_fd, &tests[i].key, str);
> + CHECK(err || memcmp(str, tests[i].str, sizeof(str)),
> + tests[i].name, "err %d result \'%s\' expected \'%s\'\n",
> + err, str, tests[i].str);
> + }
> +}
> +
> +struct foo {
> + __u8 a;
> + __u32 b;
> + __u64 c;
> +};
> +
> +static void test_global_data_struct(struct bpf_object *obj, __u32 duration)
> +{
> + int i, err, map_fd;
> + struct foo val;
> +
> + map_fd = bpf_find_map(__func__, obj, "result_struct");
> + if (map_fd < 0) {
> + error_cnt++;
> + return;
> + }
> +
> + struct {
> + char *name;
> + uint32_t key;
> + struct foo val;
> + } tests[] = {
> + { "relocate .rodata reference", 0, { 42, 0xfefeefef, 0x1111111111111111ULL, } },
> + { "relocate .bss reference", 1, { } },
> + { "relocate .rodata reference", 2, { } },
> + { "relocate .data reference", 3, { 41, 0xeeeeefef, 0x2111111111111111ULL, } },
> + };
> +
> + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
> + err = bpf_map_lookup_elem(map_fd, &tests[i].key, &val);
> + CHECK(err || memcmp(&val, &tests[i].val, sizeof(val)),
> + tests[i].name, "err %d result { %u, %u, %llu } expected { %u, %u, %llu }\n",
> + err, val.a, val.b, val.c, tests[i].val.a, tests[i].val.b, tests[i].val.c);
> + }
> +}
> +
> +static void test_global_data_rdonly(struct bpf_object *obj, __u32 duration)
> +{
> + int err = -ENOMEM, map_fd, zero = 0;
> + struct bpf_map *map;
> + __u8 *buff;
> +
> + map = bpf_object__find_map_by_name(obj, "test_glo.rodata");
> + if (!map || !bpf_map__is_internal(map)) {
> + error_cnt++;
> + return;
> + }
> +
> + map_fd = bpf_map__fd(map);
> + if (map_fd < 0) {
> + error_cnt++;
> + return;
> + }
> +
> + buff = malloc(bpf_map__def(map)->value_size);
> + if (buff)
> + err = bpf_map_update_elem(map_fd, &zero, buff, 0);
> + free(buff);
> + CHECK(!err || errno != EPERM, "test .rodata read-only map",
> + "err %d errno %d\n", err, errno);
> +}
> +
> +void test_global_data(void)
> +{
> + const char *file = "./test_global_data.o";
> + __u32 duration = 0, retval;
> + struct bpf_object *obj;
> + int err, prog_fd;
> +
> + err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
> + if (CHECK(err, "load program", "error %d loading %s\n", err, file))
> + return;
> +
> + err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
> + NULL, NULL, &retval, &duration);
> + CHECK(err || retval, "pass global data run",
> + "err %d errno %d retval %d duration %d\n",
> + err, errno, retval, duration);
> +
> + test_global_data_number(obj, duration);
> + test_global_data_string(obj, duration);
> + test_global_data_struct(obj, duration);
> + test_global_data_rdonly(obj, duration);
> +
> + bpf_object__close(obj);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_global_data.c b/tools/testing/selftests/bpf/progs/test_global_data.c
> new file mode 100644
> index 000000000000..5ab14e941980
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_global_data.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019 Isovalent, Inc.
> +
> +#include <linux/bpf.h>
> +#include <linux/pkt_cls.h>
> +#include <string.h>
> +
> +#include "bpf_helpers.h"
> +
> +struct bpf_map_def SEC("maps") result_number = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .key_size = sizeof(__u32),
> + .value_size = sizeof(__u64),
> + .max_entries = 11,
> +};
> +
> +struct bpf_map_def SEC("maps") result_string = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .key_size = sizeof(__u32),
> + .value_size = 32,
> + .max_entries = 5,
> +};
> +
> +struct foo {
> + __u8 a;
> + __u32 b;
> + __u64 c;
> +};
> +
> +struct bpf_map_def SEC("maps") result_struct = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .key_size = sizeof(__u32),
> + .value_size = sizeof(struct foo),
> + .max_entries = 5,
> +};
> +
> +/* Relocation tests for __u64s. */
> +static __u64 num0;
> +static __u64 num1 = 42;
> +static const __u64 num2 = 24;
> +static __u64 num3 = 0;
> +static __u64 num4 = 0xffeeff;
> +static const __u64 num5 = 0xabab;
> +static const __u64 num6 = 0xab;
> +
> +/* Relocation tests for strings. */
> +static const char str0[32] = "abcdefghijklmnopqrstuvwxyz";
> +static char str1[32] = "abcdefghijklmnopqrstuvwxyz";
> +static char str2[32];
> +
> +/* Relocation tests for structs. */
> +static const struct foo struct0 = {
> + .a = 42,
> + .b = 0xfefeefef,
> + .c = 0x1111111111111111ULL,
> +};
> +static struct foo struct1;
> +static const struct foo struct2;
> +static struct foo struct3 = {
> + .a = 41,
> + .b = 0xeeeeefef,
> + .c = 0x2111111111111111ULL,
> +};
> +
> +#define test_reloc(map, num, var) \
> + do { \
> + __u32 key = num; \
> + bpf_map_update_elem(&result_##map, &key, var, 0); \
> + } while (0)
> +
> +SEC("static_data_load")
> +int load_static_data(struct __sk_buff *skb)
> +{
> + static const __u64 bar = ~0;
> +
> + test_reloc(number, 0, &num0);
> + test_reloc(number, 1, &num1);
> + test_reloc(number, 2, &num2);
> + test_reloc(number, 3, &num3);
> + test_reloc(number, 4, &num4);
> + test_reloc(number, 5, &num5);
> + num4 = 1234;
> + test_reloc(number, 6, &num4);
> + test_reloc(number, 7, &num0);
> + test_reloc(number, 8, &num6);
> +
> + test_reloc(string, 0, str0);
> + test_reloc(string, 1, str1);
> + test_reloc(string, 2, str2);
> + str1[5] = 'x';
> + test_reloc(string, 3, str1);
> + __builtin_memcpy(&str2[2], "hello", sizeof("hello"));
> + test_reloc(string, 4, str2);
> +
> + test_reloc(struct, 0, &struct0);
> + test_reloc(struct, 1, &struct1);
> + test_reloc(struct, 2, &struct2);
> + test_reloc(struct, 3, &struct3);
> +
> + test_reloc(number, 9, &struct0.c);
> + test_reloc(number, 10, &bar);
> +
> + return TC_ACT_OK;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.17.1
>
