Hi,
Please see LTP changes related to bpf patches[1] posted on the linux-morello list. These patches are therefore dependent on those changes.
Some of the bpf tests require either CAP_SYS_ADMIN or CAP_BPF, or enabling unprivileged bpf with: echo "0" > /proc/sys/kernel/unprivileged_bpf_disabled
Thanks,
Zach
Review branch: https://git.morello-project.org/zdleaf/morello-linux-test-project/-/tree/mor... Kernel changes: https://git.morello-project.org/zdleaf/linux/-/tree/morello/bpf
[1] https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
Zachary Leaf (3): bpf: align tests with PCuABI/uAPI bpf: add bpf_prog_load test runtest: add bpf to extended PCuABI syscall list
include/lapi/bpf.h | 296 ++++++++++++------ runtest/morello_transitional_extended | 11 + testcases/kernel/syscalls/bpf/bpf_common.c | 10 +- testcases/kernel/syscalls/bpf/bpf_map01.c | 12 +- testcases/kernel/syscalls/bpf/bpf_prog03.c | 4 +- testcases/kernel/syscalls/bpf/bpf_prog_load.c | 108 +++++++ 6 files changed, 339 insertions(+), 102 deletions(-) create mode 100644 testcases/kernel/syscalls/bpf/bpf_prog_load.c
-- 2.34.1
PCuABI/uAPI has been updated to allow capabilities to be passed to the bpf syscall.
Align LTP tests with the new uAPI by casting pointers to uintptr_t instead of to u64. This ensures that capabilities are passed in purecap applications and remains a cast to u64 for aarch64 applications.
In addition update the union bpf_attr definition in include/lapi/bpf.h to match the kernel.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com --- include/lapi/bpf.h | 296 ++++++++++++++------- testcases/kernel/syscalls/bpf/bpf_common.c | 10 +- testcases/kernel/syscalls/bpf/bpf_map01.c | 12 +- testcases/kernel/syscalls/bpf/bpf_prog03.c | 4 +- 4 files changed, 220 insertions(+), 102 deletions(-)
diff --git a/include/lapi/bpf.h b/include/lapi/bpf.h index b44ab7d65..95768b6fc 100644 --- a/include/lapi/bpf.h +++ b/include/lapi/bpf.h @@ -14,6 +14,7 @@ #include <stdint.h>
#include "lapi/syscalls.h" +#include "linux/types.h"
/* Start copy from linux/bpf_(common).h */ #define BPF_CLASS(code) ((code) & 0x07) @@ -181,148 +182,269 @@ enum bpf_prog_type {
union bpf_attr { struct { /* anonymous struct used by BPF_MAP_CREATE command */ - uint32_t map_type; /* one of enum bpf_map_type */ - uint32_t key_size; /* size of key in bytes */ - uint32_t value_size; /* size of value in bytes */ - uint32_t max_entries; /* max number of entries in a map */ - uint32_t map_flags; /* BPF_MAP_CREATE related + __u32 map_type; /* one of enum bpf_map_type */ + __u32 key_size; /* size of key in bytes */ + __u32 value_size; /* size of value in bytes */ + __u32 max_entries; /* max number of entries in a map */ + __u32 map_flags; /* BPF_MAP_CREATE related * flags defined above. */ - uint32_t inner_map_fd; /* fd pointing to the inner map */ - uint32_t numa_node; /* numa node (effective only if + __u32 inner_map_fd; /* fd pointing to the inner map */ + __u32 numa_node; /* numa node (effective only if * BPF_F_NUMA_NODE is set). */ char map_name[BPF_OBJ_NAME_LEN]; - uint32_t map_ifindex; /* ifindex of netdev to create on */ - uint32_t btf_fd; /* fd pointing to a BTF type data */ - uint32_t btf_key_type_id; /* BTF type_id of the key */ - uint32_t btf_value_type_id; /* BTF type_id of the value */ + __u32 map_ifindex; /* ifindex of netdev to create on */ + __u32 btf_fd; /* fd pointing to a BTF type data */ + __u32 btf_key_type_id; /* BTF type_id of the key */ + __u32 btf_value_type_id; /* BTF type_id of the value */ + __u32 btf_vmlinux_value_type_id;/* BTF type_id of a kernel- + * struct stored as the + * map value + */ + /* Any per-map-type extra fields + * + * BPF_MAP_TYPE_BLOOM_FILTER - the lowest 4 bits indicate the + * number of hash functions (if 0, the bloom filter will default + * to using 5 hash functions). + */ + __u64 map_extra; };
struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */ - uint32_t map_fd; - aligned_uint64_t key; + __u32 map_fd; + __kernel_aligned_uintptr_t key; union { - aligned_uint64_t value; - aligned_uint64_t next_key; + __kernel_aligned_uintptr_t value; + __kernel_aligned_uintptr_t next_key; }; - uint64_t flags; + __u64 flags; };
+ struct { /* struct used by BPF_MAP_*_BATCH commands */ + /* start batch, NULL to start from beginning */ + __kernel_aligned_uintptr_t in_batch; + /* output: next start batch */ + __kernel_aligned_uintptr_t out_batch; + __kernel_aligned_uintptr_t keys; + __kernel_aligned_uintptr_t values; + __u32 count; /* input/output: + * input: # of key/value + * elements + * output: # of filled elements + */ + __u32 map_fd; + __u64 elem_flags; + __u64 flags; + } batch; + struct { /* anonymous struct used by BPF_PROG_LOAD command */ - uint32_t prog_type; /* one of enum bpf_prog_type */ - uint32_t insn_cnt; - aligned_uint64_t insns; - aligned_uint64_t license; - uint32_t log_level; /* verbosity level of verifier */ - uint32_t log_size; /* size of user buffer */ - aligned_uint64_t log_buf; /* user supplied buffer */ - uint32_t kern_version; /* not used */ - uint32_t prog_flags; + __u32 prog_type; /* one of enum bpf_prog_type */ + __u32 insn_cnt; + __kernel_aligned_uintptr_t insns; + __kernel_aligned_uintptr_t license; + __u32 log_level; /* verbosity level of verifier */ + __u32 log_size; /* size of user buffer */ + __kernel_aligned_uintptr_t log_buf; /* user supplied buffer */ + __u32 kern_version; /* not used */ + __u32 prog_flags; char prog_name[BPF_OBJ_NAME_LEN]; - uint32_t prog_ifindex; /* ifindex of netdev to prep for */ + __u32 prog_ifindex; /* ifindex of netdev to prep for */ /* For some prog types expected attach type must be known at * load time to verify attach type specific parts of prog * (context accesses, allowed helpers, etc). */ - uint32_t expected_attach_type; - uint32_t prog_btf_fd; /* fd pointing to BTF type data */ - uint32_t func_info_rec_size; /* userspace bpf_func_info size */ - aligned_uint64_t func_info; /* func info */ - uint32_t func_info_cnt; /* number of bpf_func_info records */ - uint32_t line_info_rec_size; /* userspace bpf_line_info size */ - aligned_uint64_t line_info; /* line info */ - uint32_t line_info_cnt; /* number of bpf_line_info records */ + __u32 expected_attach_type; + __u32 prog_btf_fd; /* fd pointing to BTF type data */ + __u32 func_info_rec_size; /* userspace bpf_func_info size */ + __kernel_aligned_uintptr_t func_info; /* func info */ + __u32 func_info_cnt; /* number of bpf_func_info records */ + __u32 line_info_rec_size; /* userspace bpf_line_info size */ + __kernel_aligned_uintptr_t line_info; /* line info */ + __u32 line_info_cnt; /* number of bpf_line_info records */ + __u32 attach_btf_id; /* in-kernel BTF type id to attach to */ + union { + /* valid prog_fd to attach to bpf prog */ + __u32 attach_prog_fd; + /* or valid module BTF object fd or 0 to attach to vmlinux */ + __u32 attach_btf_obj_fd; + }; + __u32 core_relo_cnt; /* number of bpf_core_relo */ + __kernel_aligned_uintptr_t fd_array; /* array of FDs */ + __kernel_aligned_uintptr_t core_relos; + __u32 core_relo_rec_size; /* sizeof(struct bpf_core_relo) */ };
struct { /* anonymous struct used by BPF_OBJ_* commands */ - aligned_uint64_t pathname; - uint32_t bpf_fd; - uint32_t file_flags; + __kernel_aligned_uintptr_t pathname; + __u32 bpf_fd; + __u32 file_flags; };
struct { /* anonymous struct used by BPF_PROG_ATTACH/DETACH commands */ - uint32_t target_fd; /* container object to attach to */ - uint32_t attach_bpf_fd; /* eBPF program to attach */ - uint32_t attach_type; - uint32_t attach_flags; + __u32 target_fd; /* container object to attach to */ + __u32 attach_bpf_fd; /* eBPF program to attach */ + __u32 attach_type; + __u32 attach_flags; + __u32 replace_bpf_fd; /* previously attached eBPF + * program to replace if + * BPF_F_REPLACE is used + */ };
struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */ - uint32_t prog_fd; - uint32_t retval; - uint32_t data_size_in; /* input: len of data_in */ - uint32_t data_size_out; /* input/output: len of data_out + __u32 prog_fd; + __u32 retval; + __u32 data_size_in; /* input: len of data_in */ + __u32 data_size_out; /* input/output: len of data_out * returns ENOSPC if data_out * is too small. */ - aligned_uint64_t data_in; - aligned_uint64_t data_out; - uint32_t repeat; - uint32_t duration; - uint32_t ctx_size_in; /* input: len of ctx_in */ - uint32_t ctx_size_out; /* input/output: len of ctx_out + __kernel_aligned_uintptr_t data_in; + __kernel_aligned_uintptr_t data_out; + __u32 repeat; + __u32 duration; + __u32 ctx_size_in; /* input: len of ctx_in */ + __u32 ctx_size_out; /* input/output: len of ctx_out * returns ENOSPC if ctx_out * is too small. */ - aligned_uint64_t ctx_in; - aligned_uint64_t ctx_out; + __kernel_aligned_uintptr_t ctx_in; + __kernel_aligned_uintptr_t ctx_out; + __u32 flags; + __u32 cpu; + __u32 batch_size; } test;
struct { /* anonymous struct used by BPF_*_GET_*_ID */ union { - uint32_t start_id; - uint32_t prog_id; - uint32_t map_id; - uint32_t btf_id; + __u32 start_id; + __u32 prog_id; + __u32 map_id; + __u32 btf_id; + __u32 link_id; }; - uint32_t next_id; - uint32_t open_flags; + __u32 next_id; + __u32 open_flags; };
struct { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */ - uint32_t bpf_fd; - uint32_t info_len; - aligned_uint64_t info; + __u32 bpf_fd; + __u32 info_len; + __kernel_aligned_uintptr_t info; } info;
struct { /* anonymous struct used by BPF_PROG_QUERY command */ - uint32_t target_fd; /* container object to query */ - uint32_t attach_type; - uint32_t query_flags; - uint32_t attach_flags; - aligned_uint64_t prog_ids; - uint32_t prog_cnt; + __u32 target_fd; /* container object to query */ + __u32 attach_type; + __u32 query_flags; + __u32 attach_flags; + __kernel_aligned_uintptr_t prog_ids; + __u32 prog_cnt; + /* output: per-program attach_flags. + * not allowed to be set during effective query. + */ + __kernel_aligned_uintptr_t prog_attach_flags; } query;
- struct { - uint64_t name; - uint32_t prog_fd; + struct { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */ + __kernel_aligned_uintptr_t name; + __u32 prog_fd; } raw_tracepoint;
struct { /* anonymous struct for BPF_BTF_LOAD */ - aligned_uint64_t btf; - aligned_uint64_t btf_log_buf; - uint32_t btf_size; - uint32_t btf_log_size; - uint32_t btf_log_level; + __kernel_aligned_uintptr_t btf; + __kernel_aligned_uintptr_t btf_log_buf; + __u32 btf_size; + __u32 btf_log_size; + __u32 btf_log_level; };
struct { - uint32_t pid; /* input: pid */ - uint32_t fd; /* input: fd */ - uint32_t flags; /* input: flags */ - uint32_t buf_len; /* input/output: buf len */ - aligned_uint64_t buf; /* input/output: + __u32 pid; /* input: pid */ + __u32 fd; /* input: fd */ + __u32 flags; /* input: flags */ + __u32 buf_len; /* input/output: buf len */ + __kernel_aligned_uintptr_t buf; /* input/output: * tp_name for tracepoint * symbol for kprobe * filename for uprobe */ - uint32_t prog_id; /* output: prod_id */ - uint32_t fd_type; /* output: BPF_FD_TYPE_* */ - uint64_t probe_offset; /* output: probe_offset */ - uint64_t probe_addr; /* output: probe_addr */ + __u32 prog_id; /* output: prod_id */ + __u32 fd_type; /* output: BPF_FD_TYPE_* */ + __u64 probe_offset; /* output: probe_offset */ + __u64 probe_addr; /* output: probe_addr */ } task_fd_query; + + struct { /* struct used by BPF_LINK_CREATE command */ + __u32 prog_fd; /* eBPF program to attach */ + union { + __u32 target_fd; /* object to attach to */ + __u32 target_ifindex; /* target ifindex */ + }; + __u32 attach_type; /* attach type */ + __u32 flags; /* extra flags */ + union { + __u32 target_btf_id; /* btf_id of target to attach to */ + struct { + __kernel_aligned_uintptr_t iter_info; /* extra bpf_iter_link_info */ + __u32 iter_info_len; /* iter_info length */ + }; + struct { + /* black box user-provided value passed through + * to BPF program at the execution time and + * accessible through bpf_get_attach_cookie() BPF helper + */ + __u64 bpf_cookie; + } perf_event; + struct { + __u32 flags; + __u32 cnt; + __kernel_aligned_uintptr_t syms; + __kernel_aligned_uintptr_t addrs; + __kernel_aligned_uintptr_t cookies; + } kprobe_multi; + struct { + /* this is overlaid with the target_btf_id above. */ + __u32 target_btf_id; + /* black box user-provided value passed through + * to BPF program at the execution time and + * accessible through bpf_get_attach_cookie() BPF helper + */ + __u64 cookie; + } tracing; + }; + } link_create; + + struct { /* struct used by BPF_LINK_UPDATE command */ + __u32 link_fd; /* link fd */ + /* new program fd to update link with */ + __u32 new_prog_fd; + __u32 flags; /* extra flags */ + /* expected link's program fd; is specified only if + * BPF_F_REPLACE flag is set in flags */ + __u32 old_prog_fd; + } link_update; + + struct { + __u32 link_fd; + } link_detach; + + struct { /* struct used by BPF_ENABLE_STATS command */ + __u32 type; + } enable_stats; + + struct { /* struct used by BPF_ITER_CREATE command */ + __u32 link_fd; + __u32 flags; + } iter_create; + + struct { /* struct used by BPF_PROG_BIND_MAP command */ + __u32 prog_fd; + __u32 map_fd; + __u32 flags; /* extra flags */ + } prog_bind_map; + } __attribute__((aligned(8)));
#define __BPF_FUNC_MAPPER(FN) \ @@ -613,10 +735,6 @@ enum bpf_func_id { /* End copy from tools/include/filter.h */
/* Start copy from tools/lib/bpf */ -static inline uint64_t ptr_to_u64(const void *ptr) -{ - return (uint64_t) (unsigned long) ptr; -}
static inline int bpf(enum bpf_cmd cmd, union bpf_attr *attr, unsigned int size) { diff --git a/testcases/kernel/syscalls/bpf/bpf_common.c b/testcases/kernel/syscalls/bpf/bpf_common.c index 95b5bc12e..1d2fcba61 100644 --- a/testcases/kernel/syscalls/bpf/bpf_common.c +++ b/testcases/kernel/syscalls/bpf/bpf_common.c @@ -66,8 +66,8 @@ void bpf_map_array_get(const int map_fd, { union bpf_attr elem_attr = { .map_fd = map_fd, - .key = ptr_to_u64(array_indx), - .value = ptr_to_u64(array_val), + .key = (uintptr_t)array_indx, + .value = (uintptr_t)array_val, .flags = 0 }; const int ret = bpf(BPF_MAP_LOOKUP_ELEM, &elem_attr, sizeof(elem_attr)); @@ -97,10 +97,10 @@ void bpf_init_prog_attr(union bpf_attr *const attr, memcpy(buf, prog, prog_size); memset(attr, 0, sizeof(*attr)); attr->prog_type = BPF_PROG_TYPE_SOCKET_FILTER; - attr->insns = ptr_to_u64(buf); + attr->insns = (uintptr_t)buf; attr->insn_cnt = prog_len; - attr->license = ptr_to_u64("GPL"); - attr->log_buf = ptr_to_u64(log_buf); + attr->license = (uintptr_t)"GPL"; + attr->log_buf = (uintptr_t)log_buf; attr->log_size = log_size; attr->log_level = 1; } diff --git a/testcases/kernel/syscalls/bpf/bpf_map01.c b/testcases/kernel/syscalls/bpf/bpf_map01.c index 94f9b7873..9491b256d 100644 --- a/testcases/kernel/syscalls/bpf/bpf_map01.c +++ b/testcases/kernel/syscalls/bpf/bpf_map01.c @@ -54,8 +54,8 @@ void run(unsigned int n)
memset(attr, 0, sizeof(*attr)); attr->map_fd = fd; - attr->key = ptr_to_u64(key); - attr->value = ptr_to_u64(val_get); + attr->key = (uintptr_t)key; + attr->value = (uintptr_t)val_get;
memset(val_get, 'x', VAL_SZ);
@@ -89,8 +89,8 @@ void run(unsigned int n)
memset(attr, 0, sizeof(*attr)); attr->map_fd = fd; - attr->key = ptr_to_u64(key); - attr->value = ptr_to_u64(val_set); + attr->key = (uintptr_t)key; + attr->value = (uintptr_t)val_set; attr->flags = BPF_ANY;
TEST(bpf(BPF_MAP_UPDATE_ELEM, attr, sizeof(*attr))); @@ -106,8 +106,8 @@ void run(unsigned int n)
memset(attr, 0, sizeof(*attr)); attr->map_fd = fd; - attr->key = ptr_to_u64(key); - attr->value = ptr_to_u64(val_get); + attr->key = (uintptr_t)key; + attr->value = (uintptr_t)val_get;
TEST(bpf(BPF_MAP_LOOKUP_ELEM, attr, sizeof(*attr))); if (TST_RET == -1) { diff --git a/testcases/kernel/syscalls/bpf/bpf_prog03.c b/testcases/kernel/syscalls/bpf/bpf_prog03.c index 35bb841c7..8fd5ecdaa 100644 --- a/testcases/kernel/syscalls/bpf/bpf_prog03.c +++ b/testcases/kernel/syscalls/bpf/bpf_prog03.c @@ -120,8 +120,8 @@ static void run(void)
memset(attr, 0, sizeof(*attr)); attr->map_fd = map_fd; - attr->key = ptr_to_u64(key); - attr->value = ptr_to_u64(val); + attr->key = (uintptr_t)key; + attr->value = (uintptr_t)val; attr->flags = BPF_ANY;
TEST(bpf(BPF_MAP_UPDATE_ELEM, attr, sizeof(*attr)));
Add a simple and minimal example calling only BPF_PROG_LOAD via a bpf syscall.
In addition check kernel/bpf/syscall.c:CHECK_ATTR macro implementation. The syscall should fail when we have non-zero memory in the union beyond the last element of the active sub-command struct.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com --- testcases/kernel/syscalls/bpf/bpf_prog_load.c | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 testcases/kernel/syscalls/bpf/bpf_prog_load.c
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog_load.c b/testcases/kernel/syscalls/bpf/bpf_prog_load.c new file mode 100644 index 000000000..33a9ef70e --- /dev/null +++ b/testcases/kernel/syscalls/bpf/bpf_prog_load.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) Arm Ltd. 2022. All rights reserved. + * Author: Zachary Leaf zachary.leaf@arm.com + */ + +/*\ + * minimal example, testing only BPF_PROG_LOAD option of bpf syscall + */ + +#include <limits.h> +#include <string.h> +#include <stdio.h> +#include <stddef.h> + +#include "config.h" +#include "tst_test.h" +#include "lapi/bpf.h" + +#define LOG_BUF_SIZE 4096 +static char *bpf_log_buf; +static union bpf_attr *attr; + +// as per BPF_PROG_LOAD section of man 2 bpf +int +bpf_prog_load(enum bpf_prog_type type, + const struct bpf_insn *insns, int insn_cnt) +{ + memset(attr, 0, sizeof(*attr)); + attr->prog_type = type; + attr->insns = (uintptr_t)insns; + attr->insn_cnt = insn_cnt; + attr->license = (uintptr_t)"GPL"; + attr->log_buf = (uintptr_t)bpf_log_buf; + attr->log_size = LOG_BUF_SIZE; + attr->log_level = 1; + char name[16] = "minimal_example"; + strncpy(attr->prog_name, name, 16); + + printf("sizeof(bpf_attr): %lu\n", sizeof(union bpf_attr)); + printf("insns: %#p offset:%#lx\n", (void *)attr->insns, offsetof(union bpf_attr, insns)); + printf("license: %#p offset:%#lx\n", (void *)attr->license, offsetof(union bpf_attr, license)); + printf("log_buf: %#p offset:%#lx\n", (void *)attr->log_buf, offsetof(union bpf_attr, log_buf)); + printf("prog_name: %s offset:%#lx\n", (char *)attr->prog_name, offsetof(union bpf_attr, prog_name)); + + /* test CHECK_ATTR + * check syscall fails if there is non-null data somewhere beyond + * the last struct member for the BPF_PROG_LOAD option + */ + printf("offset of core_relo_rec_size: %#lx\n", offsetof(union bpf_attr, core_relo_rec_size)); + size_t offset = offsetof(union bpf_attr, core_relo_rec_size) + + sizeof(uint32_t) + 1; + char *ptr = (char *)attr; + *(ptr+offset) = 'x'; + TST_EXP_FAIL(bpf(BPF_PROG_LOAD, attr, sizeof(*attr)), EINVAL); + + *(ptr+offset) = '\0'; + const int ret = bpf(BPF_PROG_LOAD, attr, sizeof(*attr)); + + if (ret >= 0) { + tst_res(TPASS, "Loaded program"); + return ret; + } + + if (ret != -1) + tst_brk(TBROK, "Invalid bpf() return value: %d", ret); + else + tst_brk(TBROK, "Invalid bpf() return value: %d", ret); + + if (bpf_log_buf[0] != 0) { + tst_printf("%s\n", bpf_log_buf); + tst_brk(TBROK | TERRNO, "Failed verification"); + } + + tst_brk(TBROK | TERRNO, "Failed to load program"); + return ret; +} + +void run(void) +{ + unsigned char raw_bpf[] = { + 0xb7, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x6b, 0x1a, 0xfc, 0xff, + 0x00, 0x00, 0x00, 0x00, 0xb7, 0x01, 0x00, 0x00, 0x7a, 0x6f, 0x72, 0x67, + 0x63, 0x1a, 0xf8, 0xff, 0x00, 0x00, 0x00, 0x00, 0xbf, 0xa1, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x07, 0x01, 0x00, 0x00, 0xf8, 0xff, 0xff, 0xff, + 0xb7, 0x02, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x85, 0x00, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x00, 0xb7, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + }; + unsigned int raw_bpf_len = 80; + + struct bpf_insn *insn; + insn = (struct bpf_insn*)raw_bpf; + + int prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, insn, + raw_bpf_len/sizeof(struct bpf_insn)); + SAFE_CLOSE(prog_fd); +} + +static struct tst_test test = { + .test_all = run, + .min_kver = "3.19", + .bufs = (struct tst_buffers []) { + {&bpf_log_buf, .size = LOG_BUF_SIZE}, + {&attr, .size = sizeof(*attr)}, + {}, + } +};
On 08/06/2023 10:46, Zachary Leaf wrote:
Add a simple and minimal example calling only BPF_PROG_LOAD via a bpf syscall.
In addition check kernel/bpf/syscall.c:CHECK_ATTR macro implementation. The syscall should fail when we have non-zero memory in the union beyond the last element of the active sub-command struct.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
testcases/kernel/syscalls/bpf/bpf_prog_load.c | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 testcases/kernel/syscalls/bpf/bpf_prog_load.c
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog_load.c b/testcases/kernel/syscalls/bpf/bpf_prog_load.c new file mode 100644 index 000000000..33a9ef70e --- /dev/null +++ b/testcases/kernel/syscalls/bpf/bpf_prog_load.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) Arm Ltd. 2022. All rights reserved.
- Author: Zachary Leaf zachary.leaf@arm.com
- */
+/*\
- minimal example, testing only BPF_PROG_LOAD option of bpf syscall
- */
+#include <limits.h> +#include <string.h> +#include <stdio.h> +#include <stddef.h>
+#include "config.h" +#include "tst_test.h" +#include "lapi/bpf.h"
+#define LOG_BUF_SIZE 4096 +static char *bpf_log_buf; +static union bpf_attr *attr;
+// as per BPF_PROG_LOAD section of man 2 bpf +int +bpf_prog_load(enum bpf_prog_type type,
const struct bpf_insn *insns, int insn_cnt)+{
- memset(attr, 0, sizeof(*attr));
- attr->prog_type = type;
- attr->insns = (uintptr_t)insns;
- attr->insn_cnt = insn_cnt;
- attr->license = (uintptr_t)"GPL";
- attr->log_buf = (uintptr_t)bpf_log_buf;
- attr->log_size = LOG_BUF_SIZE;
- attr->log_level = 1;
- char name[16] = "minimal_example";
- strncpy(attr->prog_name, name, 16);
Couldn't this be done using bpf_init_prog_attr()?
- printf("sizeof(bpf_attr): %lu\n", sizeof(union bpf_attr));
- printf("insns: %#p offset:%#lx\n", (void *)attr->insns, offsetof(union bpf_attr, insns));
- printf("license: %#p offset:%#lx\n", (void *)attr->license, offsetof(union bpf_attr, license));
- printf("log_buf: %#p offset:%#lx\n", (void *)attr->log_buf, offsetof(union bpf_attr, log_buf));
- printf("prog_name: %s offset:%#lx\n", (char *)attr->prog_name, offsetof(union bpf_attr, prog_name));
- /* test CHECK_ATTR
* check syscall fails if there is non-null data somewhere beyond* the last struct member for the BPF_PROG_LOAD option*/- printf("offset of core_relo_rec_size: %#lx\n", offsetof(union bpf_attr, core_relo_rec_size));
- size_t offset = offsetof(union bpf_attr, core_relo_rec_size)
+ sizeof(uint32_t) + 1;
It seems to me that LTP uses C89-style declarations like the kernel, i.e. all declarations must be at the beginning of their block (without any statement in between). However unlike the kernel -Wdeclaration-after-statement is not used, so hard to tell whether it's a hard rule or not...
- char *ptr = (char *)attr;
- *(ptr+offset) = 'x';
- TST_EXP_FAIL(bpf(BPF_PROG_LOAD, attr, sizeof(*attr)), EINVAL);
- *(ptr+offset) = '\0';
- const int ret = bpf(BPF_PROG_LOAD, attr, sizeof(*attr));
- if (ret >= 0) {
tst_res(TPASS, "Loaded program");return ret;- }
- if (ret != -1)
tst_brk(TBROK, "Invalid bpf() return value: %d", ret);- else
tst_brk(TBROK, "Invalid bpf() return value: %d", ret);
Hmm, looks like that else should be removed. I'm not sure the if is that useful either, it's essentially testing that the bpf() wrapper is implemented correctly, which is not really the point of LTP tests.
- if (bpf_log_buf[0] != 0) {
tst_printf("%s\n", bpf_log_buf);tst_brk(TBROK | TERRNO, "Failed verification");- }
- tst_brk(TBROK | TERRNO, "Failed to load program");
- return ret;
+}
+void run(void) +{
- unsigned char raw_bpf[] = {
- 0xb7, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x6b, 0x1a, 0xfc, 0xff,
- 0x00, 0x00, 0x00, 0x00, 0xb7, 0x01, 0x00, 0x00, 0x7a, 0x6f, 0x72, 0x67,
- 0x63, 0x1a, 0xf8, 0xff, 0x00, 0x00, 0x00, 0x00, 0xbf, 0xa1, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x07, 0x01, 0x00, 0x00, 0xf8, 0xff, 0xff, 0xff,
- 0xb7, 0x02, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x85, 0x00, 0x00, 0x00,
- 0x06, 0x00, 0x00, 0x00, 0xb7, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
- };
Is this just a long no-op? I'm trying to figure out what this test is adding compared to e.g. bpf_prog02, besides the CHECK_ATTR test.
- unsigned int raw_bpf_len = 80;
Is that sizeof(raw_bpf)?
- struct bpf_insn *insn;
- insn = (struct bpf_insn*)raw_bpf;
Should be just one line (declaration after statement).
- int prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, insn,
raw_bpf_len/sizeof(struct bpf_insn));- SAFE_CLOSE(prog_fd);
+}
+static struct tst_test test = {
- .test_all = run,
- .min_kver = "3.19",
Just curious, did you arrive at that version in a particular way?
Kevin
- .bufs = (struct tst_buffers []) {
{&bpf_log_buf, .size = LOG_BUF_SIZE},{&attr, .size = sizeof(*attr)},{},- }
+};
On 21/07/2023 08:19, Kevin Brodsky wrote:
On 08/06/2023 10:46, Zachary Leaf wrote:
Add a simple and minimal example calling only BPF_PROG_LOAD via a bpf syscall.
In addition check kernel/bpf/syscall.c:CHECK_ATTR macro implementation. The syscall should fail when we have non-zero memory in the union beyond the last element of the active sub-command struct.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
testcases/kernel/syscalls/bpf/bpf_prog_load.c | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 testcases/kernel/syscalls/bpf/bpf_prog_load.c
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog_load.c b/testcases/kernel/syscalls/bpf/bpf_prog_load.c new file mode 100644 index 000000000..33a9ef70e --- /dev/null +++ b/testcases/kernel/syscalls/bpf/bpf_prog_load.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) Arm Ltd. 2022. All rights reserved.
- Author: Zachary Leaf zachary.leaf@arm.com
- */
+/*\
- minimal example, testing only BPF_PROG_LOAD option of bpf syscall
- */
+#include <limits.h> +#include <string.h> +#include <stdio.h> +#include <stddef.h>
+#include "config.h" +#include "tst_test.h" +#include "lapi/bpf.h"
+#define LOG_BUF_SIZE 4096 +static char *bpf_log_buf; +static union bpf_attr *attr;
+// as per BPF_PROG_LOAD section of man 2 bpf +int +bpf_prog_load(enum bpf_prog_type type,
const struct bpf_insn *insns, int insn_cnt)+{
- memset(attr, 0, sizeof(*attr));
- attr->prog_type = type;
- attr->insns = (uintptr_t)insns;
- attr->insn_cnt = insn_cnt;
- attr->license = (uintptr_t)"GPL";
- attr->log_buf = (uintptr_t)bpf_log_buf;
- attr->log_size = LOG_BUF_SIZE;
- attr->log_level = 1;
- char name[16] = "minimal_example";
- strncpy(attr->prog_name, name, 16);
Couldn't this be done using bpf_init_prog_attr()?
Yeah - in hindsight I should have taken more time to create a proper test aligned with the other LTP tests. This was originally some experimental code I had/was using to test the basic functionality and just quickly made it work with LTP.
I've fixed it up to be more consistent with normal LTP tests i.e. using bpf_common.c and that also makes the test significantly shorter and clearer.
- printf("sizeof(bpf_attr): %lu\n", sizeof(union bpf_attr));
- printf("insns: %#p offset:%#lx\n", (void *)attr->insns, offsetof(union bpf_attr, insns));
- printf("license: %#p offset:%#lx\n", (void *)attr->license, offsetof(union bpf_attr, license));
- printf("log_buf: %#p offset:%#lx\n", (void *)attr->log_buf, offsetof(union bpf_attr, log_buf));
- printf("prog_name: %s offset:%#lx\n", (char *)attr->prog_name, offsetof(union bpf_attr, prog_name));
- /* test CHECK_ATTR
* check syscall fails if there is non-null data somewhere beyond* the last struct member for the BPF_PROG_LOAD option*/- printf("offset of core_relo_rec_size: %#lx\n", offsetof(union bpf_attr, core_relo_rec_size));
- size_t offset = offsetof(union bpf_attr, core_relo_rec_size)
+ sizeof(uint32_t) + 1;It seems to me that LTP uses C89-style declarations like the kernel, i.e. all declarations must be at the beginning of their block (without any statement in between). However unlike the kernel -Wdeclaration-after-statement is not used, so hard to tell whether it's a hard rule or not...
Ack
- char *ptr = (char *)attr;
- *(ptr+offset) = 'x';
- TST_EXP_FAIL(bpf(BPF_PROG_LOAD, attr, sizeof(*attr)), EINVAL);
- *(ptr+offset) = '\0';
- const int ret = bpf(BPF_PROG_LOAD, attr, sizeof(*attr));
- if (ret >= 0) {
tst_res(TPASS, "Loaded program");return ret;- }
- if (ret != -1)
tst_brk(TBROK, "Invalid bpf() return value: %d", ret);- else
tst_brk(TBROK, "Invalid bpf() return value: %d", ret);Hmm, looks like that else should be removed. I'm not sure the if is that useful either, it's essentially testing that the bpf() wrapper is implemented correctly, which is not really the point of LTP tests.
- if (bpf_log_buf[0] != 0) {
tst_printf("%s\n", bpf_log_buf);tst_brk(TBROK | TERRNO, "Failed verification");- }
- tst_brk(TBROK | TERRNO, "Failed to load program");
- return ret;
+}
+void run(void) +{
- unsigned char raw_bpf[] = {
- 0xb7, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x6b, 0x1a, 0xfc, 0xff,
- 0x00, 0x00, 0x00, 0x00, 0xb7, 0x01, 0x00, 0x00, 0x7a, 0x6f, 0x72, 0x67,
- 0x63, 0x1a, 0xf8, 0xff, 0x00, 0x00, 0x00, 0x00, 0xbf, 0xa1, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x07, 0x01, 0x00, 0x00, 0xf8, 0xff, 0xff, 0xff,
- 0xb7, 0x02, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x85, 0x00, 0x00, 0x00,
- 0x06, 0x00, 0x00, 0x00, 0xb7, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
- };
Is this just a long no-op? I'm trying to figure out what this test is adding compared to e.g. bpf_prog02, besides the CHECK_ATTR test.
Yeah - essentially a nop, CHECK_ATTR is the only novel part. The only other thing this test is doing is checking if it can load the simple program as above once we fix the malformed bpf_attr. This is checked implicitly by all other tests, but it's nice here just to confirm it was the non-null data that caused BPF_PROG_LOAD to fail with EINVAL.
- unsigned int raw_bpf_len = 80;
Is that sizeof(raw_bpf)?
- struct bpf_insn *insn;
- insn = (struct bpf_insn*)raw_bpf;
Should be just one line (declaration after statement).
- int prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, insn,
raw_bpf_len/sizeof(struct bpf_insn));- SAFE_CLOSE(prog_fd);
+}
+static struct tst_test test = {
- .test_all = run,
- .min_kver = "3.19",
Just curious, did you arrive at that version in a particular way?
Nope - just copied from one of the other bpf_* tests.
Now I think about it, the v2 of this test now relies on log_true_size being the last member of the BPF_PROG_LOAD struct - and this field was added in 47a71c1f9af0 ("bpf: Add log_true_size output field to return necessary log buffer size") which didn't appear until 6.4.
$ git describe --contains 47a71c1f9af0a334c9dfa97633c41de4feda4287 v6.4-rc1~77^2~118^2~19^2~7
So I've changed to 6.4 in v2.
Kevin
- .bufs = (struct tst_buffers []) {
{&bpf_log_buf, .size = LOG_BUF_SIZE},{&attr, .size = sizeof(*attr)},{},- }
+};
On 14/11/2023 15:38, Zachary Leaf wrote:
On 21/07/2023 08:19, Kevin Brodsky wrote:
On 08/06/2023 10:46, Zachary Leaf wrote:
Add a simple and minimal example calling only BPF_PROG_LOAD via a bpf syscall.
In addition check kernel/bpf/syscall.c:CHECK_ATTR macro implementation. The syscall should fail when we have non-zero memory in the union beyond the last element of the active sub-command struct.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
testcases/kernel/syscalls/bpf/bpf_prog_load.c | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 testcases/kernel/syscalls/bpf/bpf_prog_load.c
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog_load.c b/testcases/kernel/syscalls/bpf/bpf_prog_load.c new file mode 100644 index 000000000..33a9ef70e --- /dev/null +++ b/testcases/kernel/syscalls/bpf/bpf_prog_load.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) Arm Ltd. 2022. All rights reserved.
- Author: Zachary Leaf zachary.leaf@arm.com
- */
+/*\
- minimal example, testing only BPF_PROG_LOAD option of bpf syscall
- */
+#include <limits.h> +#include <string.h> +#include <stdio.h> +#include <stddef.h>
+#include "config.h" +#include "tst_test.h" +#include "lapi/bpf.h"
+#define LOG_BUF_SIZE 4096 +static char *bpf_log_buf; +static union bpf_attr *attr;
+// as per BPF_PROG_LOAD section of man 2 bpf +int +bpf_prog_load(enum bpf_prog_type type,
const struct bpf_insn *insns, int insn_cnt)+{
- memset(attr, 0, sizeof(*attr));
- attr->prog_type = type;
- attr->insns = (uintptr_t)insns;
- attr->insn_cnt = insn_cnt;
- attr->license = (uintptr_t)"GPL";
- attr->log_buf = (uintptr_t)bpf_log_buf;
- attr->log_size = LOG_BUF_SIZE;
- attr->log_level = 1;
- char name[16] = "minimal_example";
- strncpy(attr->prog_name, name, 16);
Couldn't this be done using bpf_init_prog_attr()?
Yeah - in hindsight I should have taken more time to create a proper test aligned with the other LTP tests. This was originally some experimental code I had/was using to test the basic functionality and just quickly made it work with LTP.
I've fixed it up to be more consistent with normal LTP tests i.e. using bpf_common.c and that also makes the test significantly shorter and clearer.
- printf("sizeof(bpf_attr): %lu\n", sizeof(union bpf_attr));
- printf("insns: %#p offset:%#lx\n", (void *)attr->insns, offsetof(union bpf_attr, insns));
- printf("license: %#p offset:%#lx\n", (void *)attr->license, offsetof(union bpf_attr, license));
- printf("log_buf: %#p offset:%#lx\n", (void *)attr->log_buf, offsetof(union bpf_attr, log_buf));
- printf("prog_name: %s offset:%#lx\n", (char *)attr->prog_name, offsetof(union bpf_attr, prog_name));
- /* test CHECK_ATTR
* check syscall fails if there is non-null data somewhere beyond* the last struct member for the BPF_PROG_LOAD option*/- printf("offset of core_relo_rec_size: %#lx\n", offsetof(union bpf_attr, core_relo_rec_size));
- size_t offset = offsetof(union bpf_attr, core_relo_rec_size)
+ sizeof(uint32_t) + 1;It seems to me that LTP uses C89-style declarations like the kernel, i.e. all declarations must be at the beginning of their block (without any statement in between). However unlike the kernel -Wdeclaration-after-statement is not used, so hard to tell whether it's a hard rule or not...
Ack
- char *ptr = (char *)attr;
- *(ptr+offset) = 'x';
- TST_EXP_FAIL(bpf(BPF_PROG_LOAD, attr, sizeof(*attr)), EINVAL);
- *(ptr+offset) = '\0';
- const int ret = bpf(BPF_PROG_LOAD, attr, sizeof(*attr));
- if (ret >= 0) {
tst_res(TPASS, "Loaded program");return ret;- }
- if (ret != -1)
tst_brk(TBROK, "Invalid bpf() return value: %d", ret);- else
tst_brk(TBROK, "Invalid bpf() return value: %d", ret);Hmm, looks like that else should be removed. I'm not sure the if is that useful either, it's essentially testing that the bpf() wrapper is implemented correctly, which is not really the point of LTP tests.
- if (bpf_log_buf[0] != 0) {
tst_printf("%s\n", bpf_log_buf);tst_brk(TBROK | TERRNO, "Failed verification");- }
- tst_brk(TBROK | TERRNO, "Failed to load program");
- return ret;
+}
+void run(void) +{
- unsigned char raw_bpf[] = {
- 0xb7, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x6b, 0x1a, 0xfc, 0xff,
- 0x00, 0x00, 0x00, 0x00, 0xb7, 0x01, 0x00, 0x00, 0x7a, 0x6f, 0x72, 0x67,
- 0x63, 0x1a, 0xf8, 0xff, 0x00, 0x00, 0x00, 0x00, 0xbf, 0xa1, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x07, 0x01, 0x00, 0x00, 0xf8, 0xff, 0xff, 0xff,
- 0xb7, 0x02, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x85, 0x00, 0x00, 0x00,
- 0x06, 0x00, 0x00, 0x00, 0xb7, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
- };
Is this just a long no-op? I'm trying to figure out what this test is adding compared to e.g. bpf_prog02, besides the CHECK_ATTR test.
Yeah - essentially a nop, CHECK_ATTR is the only novel part. The only other thing this test is doing is checking if it can load the simple program as above once we fix the malformed bpf_attr. This is checked implicitly by all other tests, but it's nice here just to confirm it was the non-null data that caused BPF_PROG_LOAD to fail with EINVAL.
Scratch that. I'm essentially doing the same now but with BPF_CREATE_MAP. See below.
- unsigned int raw_bpf_len = 80;
Is that sizeof(raw_bpf)?
- struct bpf_insn *insn;
- insn = (struct bpf_insn*)raw_bpf;
Should be just one line (declaration after statement).
- int prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, insn,
raw_bpf_len/sizeof(struct bpf_insn));- SAFE_CLOSE(prog_fd);
+}
+static struct tst_test test = {
- .test_all = run,
- .min_kver = "3.19",
Just curious, did you arrive at that version in a particular way?
Nope - just copied from one of the other bpf_* tests.
Now I think about it, the v2 of this test now relies on log_true_size being the last member of the BPF_PROG_LOAD struct - and this field was added in 47a71c1f9af0 ("bpf: Add log_true_size output field to return necessary log buffer size") which didn't appear until 6.4.
$ git describe --contains 47a71c1f9af0a334c9dfa97633c41de4feda4287 v6.4-rc1~77^2~118^2~19^2~7
So I've changed to 6.4 in v2.
Actually turns out this breaks the entire test, since BPF_PROG_LOAD becomes the largest sub-struct in compat64 and we then can't add anything beyond the end of it without seg-faulting.
Changed the test to use BPF_MAP_CREATE instead.
Thanks, Zach
Kevin
- .bufs = (struct tst_buffers []) {
{&bpf_log_buf, .size = LOG_BUF_SIZE},{&attr, .size = sizeof(*attr)},{},- }
+};
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
Add all bpf tests to the extended Morello transitional syscall run list.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com --- runtest/morello_transitional_extended | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/runtest/morello_transitional_extended b/runtest/morello_transitional_extended index 7c980f230..d202ecdb2 100644 --- a/runtest/morello_transitional_extended +++ b/runtest/morello_transitional_extended @@ -1,5 +1,16 @@ #DESCRIPTION: Morello transitional extended ABI system calls
+#KERN - depends on Morello Linux kernel release > morello-release-1.X.0 +bpf_map01 bpf_map01 +bpf_prog01 bpf_prog01 +bpf_prog02 bpf_prog02 +bpf_prog03 bpf_prog03 +bpf_prog04 bpf_prog04 +bpf_prog05 bpf_prog05 +bpf_prog06 bpf_prog06 +bpf_prog07 bpf_prog07 +bpf_prog_load bpf_prog_load + #KERN - depends on Morello Linux kernel release > morello-release-1.5.0 brk01 brk01 brk02 brk02
linux-morello-ltp@op-lists.linaro.org
-
Kevin Brodsky -
Zachary Leaf