diff mbox series

[RESEND,6/6] selftests/bpf: Add tests for _opts variants of bpf_*_get_fd_by_id()

Message ID 20221004131750.2306251-7-roberto.sassu@huaweicloud.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Add _opts variant for bpf_*_get_fd_by_id() | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-9 fail Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for llvm-toolchain
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-1 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_verifier on s390x with gcc

Commit Message

Roberto Sassu Oct. 4, 2022, 1:17 p.m. UTC 
From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce the data_input map, write-protected with a small eBPF program
implementing the lsm/bpf_map hook.

Then, ensure that bpf_map_get_fd_by_id() and bpf_map_get_fd_by_id_opts()
with NULL opts don't succeed due to requesting read-write access to the
write-protected map. Also, ensure that bpf_map_get_fd_by_id_opts() with
open_flags in opts set to BPF_F_RDONLY instead succeeds.

After obtaining a read-only fd, ensure that only map lookup succeeds and
not update. Ensure that update works only with the read-write fd obtained
at program loading time, when the write protection was not yet enabled.

Finally, ensure that the other _opts variants of bpf_*_get_fd_by_id() don't
work if the BPF_F_RDONLY flag is set in opts (due to the kernel not
handling the open_flags member of bpf_attr).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/testing/selftests/bpf/DENYLIST.s390x    |  1 +
 .../bpf/prog_tests/libbpf_get_fd_opts.c       | 88 +++++++++++++++++++
 .../bpf/progs/test_libbpf_get_fd_opts.c       | 36 ++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c

Comments

Andrii Nakryiko Oct. 5, 2022, 11:13 p.m. UTC | #1 
On Tue, Oct 4, 2022 at 6:19 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce the data_input map, write-protected with a small eBPF program
> implementing the lsm/bpf_map hook.
>
> Then, ensure that bpf_map_get_fd_by_id() and bpf_map_get_fd_by_id_opts()
> with NULL opts don't succeed due to requesting read-write access to the
> write-protected map. Also, ensure that bpf_map_get_fd_by_id_opts() with
> open_flags in opts set to BPF_F_RDONLY instead succeeds.
>
> After obtaining a read-only fd, ensure that only map lookup succeeds and
> not update. Ensure that update works only with the read-write fd obtained
> at program loading time, when the write protection was not yet enabled.
>
> Finally, ensure that the other _opts variants of bpf_*_get_fd_by_id() don't
> work if the BPF_F_RDONLY flag is set in opts (due to the kernel not
> handling the open_flags member of bpf_attr).
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  tools/testing/selftests/bpf/DENYLIST.s390x    |  1 +
>  .../bpf/prog_tests/libbpf_get_fd_opts.c       | 88 +++++++++++++++++++
>  .../bpf/progs/test_libbpf_get_fd_opts.c       | 36 ++++++++
>  3 files changed, 125 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c
>
> diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/selftests/bpf/DENYLIST.s390x
> index 17e074eb42b8..780a9b2090ad 100644
> --- a/tools/testing/selftests/bpf/DENYLIST.s390x
> +++ b/tools/testing/selftests/bpf/DENYLIST.s390x
> @@ -75,3 +75,4 @@ user_ringbuf                             # failed to find kernel BTF type ID of
>  lookup_key                               # JIT does not support calling kernel function                                (kfunc)
>  verify_pkcs7_sig                         # JIT does not support calling kernel function                                (kfunc)
>  kfunc_dynptr_param                       # JIT does not support calling kernel function                                (kfunc)
> +libbpf_get_fd_opts                       # failed to attach: ERROR: strerror_r(-524)=22                                (trampoline)
> diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
> new file mode 100644
> index 000000000000..2d4ba8f80d55
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
> @@ -0,0 +1,88 @@
> +// 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 "test_libbpf_get_fd_opts.skel.h"
> +
> +void test_libbpf_get_fd_opts(void)
> +{
> +       struct test_libbpf_get_fd_opts *skel;
> +       struct bpf_map_info info_m = { 0 };

1) this 0 is misleading, it's initializing only first field
(explicitly), all the other ones are implicitly initialized with
zeroes as well. In short, just use = {} to zero-initialize structs, in
general.

> +       __u32 len = sizeof(info_m), value;
> +       struct bpf_map *map;
> +       int ret, zero = 0, fd = -1;
> +
> +       DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, fd_opts_rdonly,
> +               .open_flags = BPF_F_RDONLY,
> +       );

LIBBPF_OPTS() macro defines variable, so don't add empty line before
it, it should be part of variable declaration section. Also use
shorter LIBBPF_OPTS() macro instead.


> +
> +       skel = test_libbpf_get_fd_opts__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "test_libbpf_get_fd_opts__open_and_load"))
> +               return;
> +
> +       ret = test_libbpf_get_fd_opts__attach(skel);
> +       if (!ASSERT_OK(ret, "test_libbpf_get_fd_opts__attach"))
> +               goto close_prog;
> +
> +       map = bpf_object__find_map_by_name(skel->obj, "data_input");
> +       if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name"))
> +               goto close_prog;

why find_map_by_name, you have that same map as skel->maps.data_input

> +
> +       ret = bpf_obj_get_info_by_fd(bpf_map__fd(map), &info_m, &len);
> +       if (!ASSERT_OK(ret, "bpf_obj_get_info_by_fd"))
> +               goto close_prog;

[...]

> +       /* BTF get fd with opts set should not work (no kernel support). */
> +       ret = bpf_btf_get_fd_by_id_opts(0, &fd_opts_rdonly);
> +       ASSERT_EQ(ret, -EINVAL, "bpf_btf_get_fd_by_id_opts");
> +
> +close_prog:
> +       close(fd);

if (fd >= 0) is missing

> +       test_libbpf_get_fd_opts__destroy(skel);
> +}

[...]
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/selftests/bpf/DENYLIST.s390x
index 17e074eb42b8..780a9b2090ad 100644
--- a/tools/testing/selftests/bpf/DENYLIST.s390x
+++ b/tools/testing/selftests/bpf/DENYLIST.s390x
@@ -75,3 +75,4 @@  user_ringbuf                             # failed to find kernel BTF type ID of
 lookup_key                               # JIT does not support calling kernel function                                (kfunc)
 verify_pkcs7_sig                         # JIT does not support calling kernel function                                (kfunc)
 kfunc_dynptr_param                       # JIT does not support calling kernel function                                (kfunc)
+libbpf_get_fd_opts                       # failed to attach: ERROR: strerror_r(-524)=22                                (trampoline)
diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
new file mode 100644
index 000000000000..2d4ba8f80d55
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
@@ -0,0 +1,88 @@ 
+// 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 "test_libbpf_get_fd_opts.skel.h"
+
+void test_libbpf_get_fd_opts(void)
+{
+	struct test_libbpf_get_fd_opts *skel;
+	struct bpf_map_info info_m = { 0 };
+	__u32 len = sizeof(info_m), value;
+	struct bpf_map *map;
+	int ret, zero = 0, fd = -1;
+
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, fd_opts_rdonly,
+		.open_flags = BPF_F_RDONLY,
+	);
+
+	skel = test_libbpf_get_fd_opts__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "test_libbpf_get_fd_opts__open_and_load"))
+		return;
+
+	ret = test_libbpf_get_fd_opts__attach(skel);
+	if (!ASSERT_OK(ret, "test_libbpf_get_fd_opts__attach"))
+		goto close_prog;
+
+	map = bpf_object__find_map_by_name(skel->obj, "data_input");
+	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name"))
+		goto close_prog;
+
+	ret = bpf_obj_get_info_by_fd(bpf_map__fd(map), &info_m, &len);
+	if (!ASSERT_OK(ret, "bpf_obj_get_info_by_fd"))
+		goto close_prog;
+
+	fd = bpf_map_get_fd_by_id(info_m.id);
+	if (!ASSERT_LT(fd, 0, "bpf_map_get_fd_by_id"))
+		goto close_prog;
+
+	fd = bpf_map_get_fd_by_id_opts(info_m.id, NULL);
+	if (!ASSERT_LT(fd, 0, "bpf_map_get_fd_by_id_opts"))
+		goto close_prog;
+
+	fd = bpf_map_get_fd_by_id_opts(info_m.id, &fd_opts_rdonly);
+	if (!ASSERT_GE(fd, 0, "bpf_map_get_fd_by_id_opts"))
+		goto close_prog;
+
+	/* Map lookup should work with read-only fd. */
+	ret = bpf_map_lookup_elem(fd, &zero, &value);
+	if (!ASSERT_OK(ret, "bpf_map_lookup_elem"))
+		goto close_prog;
+
+	if (!ASSERT_EQ(value, 0, "map value mismatch"))
+		goto close_prog;
+
+	/* Map update should not work with read-only fd. */
+	ret = bpf_map_update_elem(fd, &zero, &len, BPF_ANY);
+	if (!ASSERT_LT(ret, 0, "bpf_map_update_elem"))
+		goto close_prog;
+
+	/* Map update should work with read-write fd. */
+	ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &len, BPF_ANY);
+	if (!ASSERT_OK(ret, "bpf_map_update_elem"))
+		goto close_prog;
+
+	/* Prog get fd with opts set should not work (no kernel support). */
+	ret = bpf_prog_get_fd_by_id_opts(0, &fd_opts_rdonly);
+	if (!ASSERT_EQ(ret, -EINVAL, "bpf_prog_get_fd_by_id_opts"))
+		goto close_prog;
+
+	/* Link get fd with opts set should not work (no kernel support). */
+	ret = bpf_link_get_fd_by_id_opts(0, &fd_opts_rdonly);
+	if (!ASSERT_EQ(ret, -EINVAL, "bpf_link_get_fd_by_id_opts"))
+		goto close_prog;
+
+	/* BTF get fd with opts set should not work (no kernel support). */
+	ret = bpf_btf_get_fd_by_id_opts(0, &fd_opts_rdonly);
+	ASSERT_EQ(ret, -EINVAL, "bpf_btf_get_fd_by_id_opts");
+
+close_prog:
+	close(fd);
+	test_libbpf_get_fd_opts__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c b/tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c
new file mode 100644
index 000000000000..f5ac5f3e8919
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c
@@ -0,0 +1,36 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ */
+
+#include "vmlinux.h"
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+/* From include/linux/mm.h. */
+#define FMODE_WRITE	0x2
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u32);
+} data_input SEC(".maps");
+
+char _license[] SEC("license") = "GPL";
+
+SEC("lsm/bpf_map")
+int BPF_PROG(check_access, struct bpf_map *map, fmode_t fmode)
+{
+	if (map != (struct bpf_map *)&data_input)
+		return 0;
+
+	if (fmode & FMODE_WRITE)
+		return -EACCES;
+
+	return 0;
+}