On Thu, Jun 2, 2022 at 7:39 AM Roberto Sassu roberto.sassu@huawei.com wrote:
Add some tests to ensure that read-like operations can be performed on a write-protected map, and that write-like operations fail with a read file descriptor.
Do the tests programmatically, with the new functions bpf_map_get_fd_by_id_flags() and bpf_obj_get_flags(), added to libbpf, and with the bpftool binary.
Also ensure that map search by name works when there is a write-protected map. Before, iteration over existing maps stopped due to not being able to get a file descriptor with full permissions.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
.../bpf/prog_tests/test_map_check_access.c | 264 ++++++++++++++++++ .../selftests/bpf/progs/map_check_access.c | 65 +++++ 2 files changed, 329 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/test_map_check_access.c create mode 100644 tools/testing/selftests/bpf/progs/map_check_access.c
general convention (not universally followed, unfortunately) is opposite, progs/test_<something>.c and prog_tests/<something>.c. This allows to not have weird test_test_<something> naming pattern
diff --git a/tools/testing/selftests/bpf/prog_tests/test_map_check_access.c b/tools/testing/selftests/bpf/prog_tests/test_map_check_access.c new file mode 100644 index 000000000000..20ccadcdf10f --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/test_map_check_access.c @@ -0,0 +1,264 @@ +// SPDX-License-Identifier: GPL-2.0
+/*
- Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
- Author: Roberto Sassu roberto.sassu@huawei.com
- */
+#include <test_progs.h>
+#include "map_check_access.skel.h"
+#define PINNED_MAP_PATH "/sys/fs/bpf/test_map_check_access_map" +#define BPFTOOL_PATH "./tools/build/bpftool/bpftool"
+enum check_types { CHECK_NONE, CHECK_PINNED, CHECK_METADATA };
+static int populate_argv(char *argv[], int max_args, char *cmdline) +{
char *arg;int i = 0;argv[i++] = BPFTOOL_PATH;while ((arg = strsep(&cmdline, " "))) {if (i == max_args - 1)break;argv[i++] = arg;}argv[i] = NULL;return i;+}
+static void restore_cmdline(char *argv[], int num_args) +{
int i;for (i = 1; i < num_args - 1; i++)argv[i][strlen(argv[i])] = ' ';+}
I'm missing the point of this cmdline restoration?..
+static int _run_bpftool(char *cmdline, enum check_types check) +{
char *argv[20];char output[1024];int ret, fd[2], num_args, child_pid, child_status;num_args = populate_argv(argv, ARRAY_SIZE(argv), cmdline);ret = pipe(fd);if (ret < 0)return ret;
and popen() doesn't work instead of all this forking/execve sequence?
child_pid = fork();if (child_pid == 0) {close(fd[0]);close(STDOUT_FILENO);close(STDERR_FILENO);ret = dup2(fd[1], STDOUT_FILENO);if (ret < 0) {close(fd[1]);exit(errno);}execv(BPFTOOL_PATH, argv);close(fd[1]);exit(errno);} else if (child_pid > 0) {close(fd[1]);restore_cmdline(argv, num_args);waitpid(child_pid, &child_status, 0);if (WEXITSTATUS(child_status)) {close(fd[0]);return WEXITSTATUS(child_status);}ret = read(fd[0], output, sizeof(output) - 1);close(fd[0]);if (ret < 0)return ret;output[ret] = '\0';ret = 0;switch (check) {case CHECK_PINNED:if (!strstr(output, PINNED_MAP_PATH))ret = -ENOENT;break;case CHECK_METADATA:if (!strstr(output, "test_var"))ret = -ENOENT;break;default:break;}return ret;}close(fd[0]);close(fd[1]);return -EINVAL;+}
+void test_test_map_check_access(void) +{
struct map_check_access *skel;struct bpf_map_info info_m = { 0 };struct bpf_map *map;__u32 len = sizeof(info_m);char cmdline[1024];int ret, zero = 0, fd, duration = 0;skel = map_check_access__open_and_load();if (CHECK(!skel, "skel", "open_and_load failed\n"))goto close_prog;ret = map_check_access__attach(skel);if (CHECK(ret < 0, "skel", "attach failed\n"))goto close_prog;
please don't use CHECK(), use ASSERT_xxx() macros instead
map = bpf_object__find_map_by_name(skel->obj, "data_input");if (CHECK(!map, "bpf_object__find_map_by_name", "not found\n"))goto close_prog;ret = bpf_obj_get_info_by_fd(bpf_map__fd(map), &info_m, &len);if (CHECK(ret < 0, "bpf_obj_get_info_by_fd", "error: %d\n", ret))goto close_prog;fd = bpf_map_get_fd_by_id(info_m.id);if (CHECK(fd >= 0, "bpf_map_get_fd_by_id","should fail (map write-protected)\n"))goto close_prog;
[...]
snprintf(cmdline, sizeof(cmdline), "btf dump map name data_input");ret = _run_bpftool(cmdline, CHECK_NONE);if (CHECK(ret, "bpftool", "%s - error: %d\n", cmdline, ret))goto close_prog;snprintf(cmdline, sizeof(cmdline), "map pin name data_input %s",PINNED_MAP_PATH);ret = _run_bpftool(cmdline, CHECK_NONE);if (CHECK(ret, "bpftool", "%s - error: %d\n", cmdline, ret))goto close_prog;snprintf(cmdline, sizeof(cmdline), "struct_ops show name dummy_2");ret = _run_bpftool(cmdline, CHECK_NONE);
if you don't have to restore anything, you don't need snprintf()'ing, just pass arguments as a string literal directly
if (CHECK(ret, "bpftool", "%s - error: %d\n", cmdline, ret))goto close_prog;snprintf(cmdline, sizeof(cmdline), "struct_ops dump name dummy_2");ret = _run_bpftool(cmdline, CHECK_NONE);CHECK(ret, "_run_bpftool", "%s - error: %d\n", cmdline, ret);+close_prog:
map_check_access__destroy(skel);unlink(PINNED_MAP_PATH);+}
[...]