| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c70c0a07-a337-1710-fae7-41ab77f75544@fb.com>
Date: Thu, 16 Sep 2021 16:46:33 -0700
From: Yonghong Song <yhs@...com>
To: Hou Tao <houtao1@...wei.com>, Alexei Starovoitov <ast@...nel.org>
CC: Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, <bpf@...r.kernel.org>,
<netdev@...r.kernel.org>
Subject: Re: [PATCH 3/3] bpf/selftests: add test for writable bare tracepoint
On 9/16/21 6:55 AM, Hou Tao wrote:
> Add a writable bare tracepoint in bpf_testmod module, and
> trigger its calling when reading /sys/kernel/bpf_testmod
> with a specific buffer length.
The patch cannot be applied cleanly with bpf-next tree.
Please rebase and resubmit.
>
> Signed-off-by: Hou Tao <houtao1@...wei.com>
> ---
> .../bpf/bpf_testmod/bpf_testmod-events.h | 15 +++++++
> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +++++
> .../selftests/bpf/bpf_testmod/bpf_testmod.h | 5 +++
> .../selftests/bpf/prog_tests/module_attach.c | 40 ++++++++++++++++++-
> .../selftests/bpf/progs/test_module_attach.c | 14 +++++++
> 5 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
> index 89c6d58e5dd6..11ee801e75e7 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
> @@ -34,6 +34,21 @@ DECLARE_TRACE(bpf_testmod_test_write_bare,
> TP_ARGS(task, ctx)
> );
>
> +#undef BPF_TESTMOD_DECLARE_TRACE
> +#ifdef DECLARE_TRACE_WRITABLE
> +#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
> + DECLARE_TRACE_WRITABLE(call, PARAMS(proto), PARAMS(args), size)
> +#else
> +#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
> + DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))
> +#endif
> +
> +BPF_TESTMOD_DECLARE_TRACE(bpf_testmod_test_writable_bare,
> + TP_PROTO(struct bpf_testmod_test_writable_ctx *ctx),
> + TP_ARGS(ctx),
> + sizeof(struct bpf_testmod_test_writable_ctx)
> +);
> +
> #endif /* _BPF_TESTMOD_EVENTS_H */
>
> #undef TRACE_INCLUDE_PATH
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 141d8da687d2..3d3fb16eaf8c 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -26,6 +26,16 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
>
> trace_bpf_testmod_test_read(current, &ctx);
>
> + /* Magic number to enable writable tp */
> + if (len == 1024) {
> + struct bpf_testmod_test_writable_ctx writable = {
> + .val = 1024,
> + };
> + trace_bpf_testmod_test_writable_bare(&writable);
> + if (writable.ret)
> + return snprintf(buf, len, "%d\n", writable.val);
> + }
> +
> return -EIO; /* always fail */
> }
> EXPORT_SYMBOL(bpf_testmod_test_read);
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> index b3892dc40111..553d94214aa6 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> @@ -17,4 +17,9 @@ struct bpf_testmod_test_write_ctx {
> size_t len;
> };
>
> +struct bpf_testmod_test_writable_ctx {
> + bool ret;
> + int val;
> +};
> +
> #endif /* _BPF_TESTMOD_H */
> diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> index d85a69b7ce44..5565bcab1531 100644
> --- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> @@ -6,11 +6,39 @@
>
> static int duration;
>
> +#define BPF_TESTMOD_PATH "/sys/kernel/bpf_testmod"
> +
> +static int trigger_module_test_writable(int *val)
> +{
> + int fd, err;
> + char buf[1025];
Not critical, but do you need such a big stack size?
Maybe smaller one?
> + ssize_t rd;
> +
> + fd = open(BPF_TESTMOD_PATH, O_RDONLY);
> + err = -errno;
> + if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
> + return err;
> +
> + rd = read(fd, buf, sizeof(buf) - 1);
> + err = rd < 0 ? -errno : -EIO;
> + if (CHECK(rd <= 0, "testmod_file_read", "failed: %d\n", err)) {
Please use ASSERT_* macros. You can take a look at other self tests.
> + close(fd);
> + return err;
> + }
Put one blank line here and remove the following three blank lines.
> + buf[rd] = '\0';
> +
> + *val = strtol(buf, NULL, 0);
> +
> + close(fd);
> +
> + return 0;
> +}
> +
> static int trigger_module_test_read(int read_sz)
> {
> int fd, err;
>
> - fd = open("/sys/kernel/bpf_testmod", O_RDONLY);
> + fd = open(BPF_TESTMOD_PATH, O_RDONLY);
> err = -errno;
> if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
> return err;
> @@ -32,7 +60,7 @@ static int trigger_module_test_write(int write_sz)
> memset(buf, 'a', write_sz);
> buf[write_sz-1] = '\0';
>
> - fd = open("/sys/kernel/bpf_testmod", O_WRONLY);
> + fd = open(BPF_TESTMOD_PATH, O_WRONLY);
> err = -errno;
> if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err)) {
> free(buf);
> @@ -58,6 +86,7 @@ void test_module_attach(void)
> struct test_module_attach__bss *bss;
> struct bpf_link *link;
> int err;
> + int writable_val;
>
> skel = test_module_attach__open();
> if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> @@ -90,6 +119,13 @@ void test_module_attach(void)
> ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
> ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
>
> + bss->raw_tp_writable_bare_ret = 1;
> + bss->raw_tp_writable_bare_val = 511;
> + writable_val = 0;
> + ASSERT_OK(trigger_module_test_writable(&writable_val), "trigger_writable");
> + ASSERT_EQ(bss->raw_tp_writable_bare_in_val, 1024, "writable_test");
> + ASSERT_EQ(bss->raw_tp_writable_bare_val, writable_val, "writable_test");
> +
> test_module_attach__detach(skel);
>
> /* attach fentry/fexit and make sure it get's module reference */
> diff --git a/tools/testing/selftests/bpf/progs/test_module_attach.c b/tools/testing/selftests/bpf/progs/test_module_attach.c
> index bd37ceec5587..4f5c780fcd21 100644
> --- a/tools/testing/selftests/bpf/progs/test_module_attach.c
> +++ b/tools/testing/selftests/bpf/progs/test_module_attach.c
> @@ -27,6 +27,20 @@ int BPF_PROG(handle_raw_tp_bare,
> return 0;
> }
>
> +int raw_tp_writable_bare_in_val = 0;
> +int raw_tp_writable_bare_ret = 0;
> +int raw_tp_writable_bare_val = 0;
> +
> +SEC("raw_tp_writable/bpf_testmod_test_writable_bare")
> +int BPF_PROG(handle_raw_tp_writable_bare,
> + struct bpf_testmod_test_writable_ctx *writable)
> +{
> + raw_tp_writable_bare_in_val = writable->val;
> + writable->ret = raw_tp_writable_bare_ret;
> + writable->val = raw_tp_writable_bare_val;
> + return 0;
> +}
> +
> __u32 tp_btf_read_sz = 0;
>
> SEC("tp_btf/bpf_testmod_test_read")
>
Powered by blists - more mailing lists