From: Yonghong Song <yonghong.song@linux.dev>
To: Juntong Deng <juntong.deng@outlook.com>,
ast@kernel.org, daniel@iogearbox.net, john.fastabend@gmail.com,
andrii@kernel.org, martin.lau@linux.dev, eddyz87@gmail.com,
song@kernel.org, kpsingh@kernel.org, sdf@fomichev.me,
haoluo@google.com, jolsa@kernel.org, memxor@gmail.com,
snorcht@gmail.com, brauner@kernel.org
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH bpf-next v6 2/5] selftests/bpf: Add tests for open-coded style process file iterator
Date: Thu, 19 Dec 2024 08:35:09 -0800 [thread overview]
Message-ID: <2d389258-ad08-4d28-a347-667909b0e190@linux.dev> (raw)
In-Reply-To: <AM6PR03MB508014EBAC89D14D0C3ADA6899042@AM6PR03MB5080.eurprd03.prod.outlook.com>
On 12/17/24 3:37 PM, Juntong Deng wrote:
> This patch adds test cases for open-coded style process file iterator.
>
> Test cases related to process files are run in the newly created child
> process. Close all opened files inherited from the parent process in
> the child process to avoid the files opened by the parent process
> affecting the test results.
>
> In addition, this patch adds failure test cases where bpf programs
> cannot pass the verifier due to uninitialized or untrusted
> arguments, etc.
>
> Signed-off-by: Juntong Deng <juntong.deng@outlook.com>
> ---
> .../testing/selftests/bpf/bpf_experimental.h | 7 ++
> .../testing/selftests/bpf/prog_tests/iters.c | 79 ++++++++++++++++
> .../selftests/bpf/progs/iters_task_file.c | 86 ++++++++++++++++++
> .../bpf/progs/iters_task_file_failure.c | 91 +++++++++++++++++++
> 4 files changed, 263 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/iters_task_file.c
> create mode 100644 tools/testing/selftests/bpf/progs/iters_task_file_failure.c
>
> diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
> index cd8ecd39c3f3..ce1520c56b55 100644
> --- a/tools/testing/selftests/bpf/bpf_experimental.h
> +++ b/tools/testing/selftests/bpf/bpf_experimental.h
> @@ -588,4 +588,11 @@ extern int bpf_iter_kmem_cache_new(struct bpf_iter_kmem_cache *it) __weak __ksym
> extern struct kmem_cache *bpf_iter_kmem_cache_next(struct bpf_iter_kmem_cache *it) __weak __ksym;
> extern void bpf_iter_kmem_cache_destroy(struct bpf_iter_kmem_cache *it) __weak __ksym;
>
> +struct bpf_iter_task_file;
> +struct bpf_iter_task_file_item;
> +extern int bpf_iter_task_file_new(struct bpf_iter_task_file *it, struct task_struct *task) __ksym;
> +extern struct bpf_iter_task_file_item *
> +bpf_iter_task_file_next(struct bpf_iter_task_file *it) __ksym;
> +extern void bpf_iter_task_file_destroy(struct bpf_iter_task_file *it) __ksym;
All the above declarations should be in vmlinux.h already and I see your below bpf prog already
included vmlinux.h, there is no need to put them here.
> +
> #endif
> diff --git a/tools/testing/selftests/bpf/prog_tests/iters.c b/tools/testing/selftests/bpf/prog_tests/iters.c
> index 3cea71f9c500..cfe5b56cc027 100644
> --- a/tools/testing/selftests/bpf/prog_tests/iters.c
> +++ b/tools/testing/selftests/bpf/prog_tests/iters.c
> @@ -1,6 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
>
> +#define _GNU_SOURCE
> +#include <sys/socket.h>
> #include <sys/syscall.h>
> #include <sys/mman.h>
> #include <sys/wait.h>
> @@ -16,11 +18,13 @@
> #include "iters_num.skel.h"
> #include "iters_testmod.skel.h"
> #include "iters_testmod_seq.skel.h"
> +#include "iters_task_file.skel.h"
> #include "iters_task_vma.skel.h"
> #include "iters_task.skel.h"
> #include "iters_css_task.skel.h"
> #include "iters_css.skel.h"
> #include "iters_task_failure.skel.h"
> +#include "iters_task_file_failure.skel.h"
>
> static void subtest_num_iters(void)
> {
> @@ -291,6 +295,78 @@ static void subtest_css_iters(void)
> iters_css__destroy(skel);
> }
>
> +static int task_file_test_process(void *args)
> +{
> + int pipefd[2], sockfd, err = 0;
> +
> + /* Create a clean file descriptor table for the test process */
> + close_range(0, ~0U, 0);
> +
> + if (pipe(pipefd) < 0)
> + return 1;
> +
> + sockfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> + if (sockfd < 0) {
> + err = 2;
> + goto cleanup_pipe;
> + }
> +
> + usleep(1);
> +
> + close(sockfd);
> +cleanup_pipe:
> + close(pipefd[0]);
> + close(pipefd[1]);
> + return err;
> +}
> +
> +static void subtest_task_file_iters(void)
> +{
> + const int stack_size = 1024 * 1024;
> + struct iters_task_file *skel;
> + int child_pid, wstatus, err;
> + char *stack;
> +
> + skel = iters_task_file__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "open_and_load"))
> + return;
> +
> + if (!ASSERT_OK(skel->bss->err, "pre_test_err"))
> + goto cleanup_skel;
> +
> + skel->bss->parent_pid = getpid();
> + skel->bss->count = 0;
> +
> + err = iters_task_file__attach(skel);
> + if (!ASSERT_OK(err, "skel_attach"))
> + goto cleanup_skel;
> +
> + stack = (char *)malloc(stack_size);
> + if (!ASSERT_OK_PTR(stack, "clone_stack"))
> + goto cleanup_attach;
> +
> + /* Note that there is no CLONE_FILES */
> + child_pid = clone(task_file_test_process, stack + stack_size, CLONE_VM | SIGCHLD, NULL);
> + if (!ASSERT_GT(child_pid, -1, "child_pid"))
> + goto cleanup_stack;
> +
> + if (!ASSERT_GT(waitpid(child_pid, &wstatus, 0), -1, "waitpid"))
> + goto cleanup_stack;
> +
> + if (!ASSERT_OK(WEXITSTATUS(wstatus), "run_task_file_iters_test_err"))
> + goto cleanup_stack;
> +
> + ASSERT_EQ(skel->bss->count, 1, "run_task_file_iters_test_count_err");
> + ASSERT_OK(skel->bss->err, "run_task_file_iters_test_failure");
> +
> +cleanup_stack:
> + free(stack);
> +cleanup_attach:
> + iters_task_file__detach(skel);
> +cleanup_skel:
> + iters_task_file__destroy(skel);
> +}
> +
> void test_iters(void)
> {
> RUN_TESTS(iters_state_safety);
> @@ -315,5 +391,8 @@ void test_iters(void)
> subtest_css_task_iters();
> if (test__start_subtest("css"))
> subtest_css_iters();
> + if (test__start_subtest("task_file"))
> + subtest_task_file_iters();
> RUN_TESTS(iters_task_failure);
> + RUN_TESTS(iters_task_file_failure);
> }
> diff --git a/tools/testing/selftests/bpf/progs/iters_task_file.c b/tools/testing/selftests/bpf/progs/iters_task_file.c
> new file mode 100644
> index 000000000000..47941530e51b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/iters_task_file.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +#include "bpf_experimental.h"
> +#include "task_kfunc_common.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +int err, parent_pid, count;
> +
> +extern const void pipefifo_fops __ksym;
> +extern const void socket_file_ops __ksym;
There is no need to have 'const' in the above two extern declarations.
> +
> +SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> +int test_bpf_iter_task_file(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> + struct bpf_iter_task_file_item *item;
> + struct task_struct *task;
> +
> + task = bpf_get_current_task_btf();
> + if (task->parent->pid != parent_pid)
> + return 0;
> +
> + count++;
> +
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item == NULL) {
> + err = 1;
> + goto cleanup;
> + }
> +
> + if (item->fd != 0) {
> + err = 2;
> + goto cleanup;
> + }
> +
> + if (item->file->f_op != &pipefifo_fops) {
> + err = 3;
> + goto cleanup;
> + }
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item == NULL) {
> + err = 4;
> + goto cleanup;
> + }
> +
> + if (item->fd != 1) {
> + err = 5;
> + goto cleanup;
> + }
> +
> + if (item->file->f_op != &pipefifo_fops) {
> + err = 6;
> + goto cleanup;
> + }
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item == NULL) {
> + err = 7;
> + goto cleanup;
> + }
> +
> + if (item->fd != 2) {
> + err = 8;
> + goto cleanup;
> + }
> +
> + if (item->file->f_op != &socket_file_ops) {
> + err = 9;
> + goto cleanup;
> + }
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item != NULL)
> + err = 10;
> +cleanup:
> + bpf_iter_task_file_destroy(&task_file_it);
> + return 0;
> +}
[...]
next prev parent reply other threads:[~2024-12-19 16:35 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-17 23:34 [PATCH bpf-next v6 0/5] bpf: Add open-coded style process file iterator and bpf_fget_task() kfunc Juntong Deng
2024-12-17 23:37 ` [PATCH bpf-next v6 1/5] bpf: Introduce task_file open-coded iterator kfuncs Juntong Deng
2024-12-19 16:19 ` Yonghong Song
2024-12-17 23:37 ` [PATCH bpf-next v6 2/5] selftests/bpf: Add tests for open-coded style process file iterator Juntong Deng
2024-12-19 16:35 ` Yonghong Song [this message]
2024-12-24 0:43 ` Juntong Deng
2024-12-17 23:37 ` [PATCH bpf-next v6 3/5] bpf: Add bpf_fget_task() kfunc Juntong Deng
2024-12-17 23:37 ` [PATCH bpf-next v6 4/5] bpf: Make fs kfuncs available for SYSCALL program type Juntong Deng
2024-12-19 16:41 ` Alexei Starovoitov
2024-12-24 0:51 ` Juntong Deng
2024-12-24 12:10 ` Juntong Deng
2025-01-09 19:23 ` per st_ops kfunc allow/deny mask. Was: " Alexei Starovoitov
2025-01-09 20:49 ` Song Liu
2025-01-09 21:24 ` Alexei Starovoitov
2025-01-10 20:19 ` Tejun Heo
2025-01-10 20:50 ` Juntong Deng
2025-01-10 20:42 ` Juntong Deng
2025-01-16 19:52 ` Juntong Deng
2024-12-17 23:37 ` [PATCH bpf-next v6 5/5] selftests/bpf: Add tests for bpf_fget_task() kfunc Juntong Deng
2024-12-19 16:11 ` [PATCH bpf-next v6 0/5] bpf: Add open-coded style process file iterator and " Yonghong Song
2024-12-24 0:42 ` Juntong Deng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2d389258-ad08-4d28-a347-667909b0e190@linux.dev \
--to=yonghong.song@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brauner@kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=juntong.deng@outlook.com \
--cc=kpsingh@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=sdf@fomichev.me \
--cc=snorcht@gmail.com \
--cc=song@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox