[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zz3HjT24glXY-8AF@krava>
Date: Wed, 20 Nov 2024 12:27:09 +0100
From: Jiri Olsa <olsajiri@...il.com>
To: Juntong Deng <juntong.deng@...look.com>
Cc: ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com,
andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com,
song@...nel.org, yonghong.song@...ux.dev, kpsingh@...nel.org,
sdf@...ichev.me, haoluo@...gle.com, memxor@...il.com,
snorcht@...il.com, brauner@...nel.org, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 2/5] selftests/bpf: Add tests for open-coded
style process file iterator
On Tue, Nov 19, 2024 at 05:53:59PM +0000, Juntong Deng wrote:
SNIP
> +static void subtest_task_file_iters(void)
> +{
> + int prog_fd, child_pid, wstatus, err = 0;
> + const int stack_size = 1024 * 1024;
> + struct iters_task_file *skel;
> + struct files_test_args args;
> + struct bpf_program *prog;
> + bool setup_end, test_end;
> + 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;
> +
> + prog = bpf_object__find_program_by_name(skel->obj, "test_bpf_iter_task_file");
> + if (!ASSERT_OK_PTR(prog, "find_program_by_name"))
> + goto cleanup_skel;
> +
> + prog_fd = bpf_program__fd(prog);
> + if (!ASSERT_GT(prog_fd, -1, "bpf_program__fd"))
> + goto cleanup_skel;
I don't think you need to check on this once we did iters_task_file__open_and_load
> +
> + stack = (char *)malloc(stack_size);
> + if (!ASSERT_OK_PTR(stack, "clone_stack"))
> + goto cleanup_skel;
> +
> + setup_end = false;
> + test_end = false;
> +
> + args.setup_end = &setup_end;
> + args.test_end = &test_end;
> +
> + /* Note that there is no CLONE_FILES */
> + child_pid = clone(task_file_test_process, stack + stack_size, CLONE_VM | SIGCHLD, &args);
> + if (!ASSERT_GT(child_pid, -1, "child_pid"))
> + goto cleanup_stack;
> +
> + while (!setup_end)
> + ;
I thin kthe preferred way is to synchronize through pipe,
you can check prog_tests/uprobe_multi_test.c
> +
> + skel->bss->pid = child_pid;
> +
> + err = bpf_prog_test_run_opts(prog_fd, NULL);
> + if (!ASSERT_OK(err, "prog_test_run"))
> + goto cleanup_stack;
> +
> + test_end = true;
> +
> + 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_OK(skel->bss->err, "run_task_file_iters_test_failure");
could the test check on that the iterated files were (or contained) the ones we expected?
thanks,
jirka
> +cleanup_stack:
> + free(stack);
> +cleanup_skel:
> + iters_task_file__destroy(skel);
> +}
> +
> void test_iters(void)
> {
> RUN_TESTS(iters_state_safety);
> @@ -315,5 +417,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..f14b473936c7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/iters_task_file.c
> @@ -0,0 +1,71 @@
> +// 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, pid;
> +
> +SEC("syscall")
> +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_task_from_vpid(pid);
> + if (task == NULL) {
> + err = 1;
> + return 0;
> + }
> +
> + bpf_rcu_read_lock();
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item == NULL) {
> + err = 2;
> + goto cleanup;
> + }
> +
> + if (item->fd != 0) {
> + 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;
> + }
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item == NULL) {
> + err = 6;
> + goto cleanup;
> + }
> +
> + if (item->fd != 2) {
> + err = 7;
> + goto cleanup;
> + }
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item != NULL)
> + err = 8;
> +cleanup:
> + bpf_iter_task_file_destroy(&task_file_it);
> + bpf_rcu_read_unlock();
> + bpf_task_release(task);
> + return 0;
> +}
> diff --git a/tools/testing/selftests/bpf/progs/iters_task_file_failure.c b/tools/testing/selftests/bpf/progs/iters_task_file_failure.c
> new file mode 100644
> index 000000000000..c3de9235b888
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/iters_task_file_failure.c
> @@ -0,0 +1,114 @@
> +// 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";
> +
> +SEC("syscall")
> +__failure __msg("expected an RCU CS when using bpf_iter_task_file")
> +int bpf_iter_task_file_new_without_rcu_lock(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> + struct task_struct *task;
> +
> + task = bpf_get_current_task_btf();
> +
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + bpf_iter_task_file_destroy(&task_file_it);
> + return 0;
> +}
> +
> +SEC("syscall")
> +__failure __msg("expected uninitialized iter_task_file as arg #1")
> +int bpf_iter_task_file_new_inited_iter(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> + struct task_struct *task;
> +
> + task = bpf_get_current_task_btf();
> +
> + bpf_rcu_read_lock();
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + bpf_iter_task_file_destroy(&task_file_it);
> + bpf_rcu_read_unlock();
> + return 0;
> +}
> +
> +SEC("syscall")
> +__failure __msg("Possibly NULL pointer passed to trusted arg1")
> +int bpf_iter_task_file_new_null_task(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> + struct task_struct *task = NULL;
> +
> + bpf_rcu_read_lock();
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + bpf_iter_task_file_destroy(&task_file_it);
> + bpf_rcu_read_unlock();
> + return 0;
> +}
> +
> +SEC("syscall")
> +__failure __msg("R2 must be referenced or trusted")
> +int bpf_iter_task_file_new_untrusted_task(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> + struct task_struct *task;
> +
> + task = bpf_get_current_task_btf()->parent;
> +
> + bpf_rcu_read_lock();
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + bpf_iter_task_file_destroy(&task_file_it);
> + bpf_rcu_read_unlock();
> + return 0;
> +}
> +
> +SEC("syscall")
> +__failure __msg("Unreleased reference")
> +int bpf_iter_task_file_no_destory(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> + struct task_struct *task;
> +
> + task = bpf_get_current_task_btf();
> +
> + bpf_rcu_read_lock();
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + bpf_rcu_read_unlock();
> + return 0;
> +}
> +
> +SEC("syscall")
> +__failure __msg("expected an initialized iter_task_file as arg #1")
> +int bpf_iter_task_file_next_uninit_iter(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> +
> + bpf_iter_task_file_next(&task_file_it);
> +
> + return 0;
> +}
> +
> +SEC("syscall")
> +__failure __msg("expected an initialized iter_task_file as arg #1")
> +int bpf_iter_task_file_destroy_uninit_iter(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> +
> + bpf_iter_task_file_destroy(&task_file_it);
> +
> + return 0;
> +}
> --
> 2.39.5
>
Powered by blists - more mailing lists