| 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:
<AM6PR03MB5080FB540DD0BBFA48C20325992F2@AM6PR03MB5080.eurprd03.prod.outlook.com>
Date: Tue, 26 Nov 2024 22:24:07 +0000
From: Juntong Deng <juntong.deng@...look.com>
To: Jiri Olsa <olsajiri@...il.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 2024/11/20 11:27, Jiri Olsa wrote:
> 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
>
Thanks for your reply.
Do we really need to use pipe? Currently this test is very simple.
In this test, all files opened by the test process will be closed first
so that there is an empty file description table, and then open the
test files.
This way the test process has only 3 newly opened files and the file
descriptors are always 0, 1, 2.
Although using pipe is feasible, this test will become more complicated
than it is now.
>> +
>> + 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?
>
Yes, that is the purpose of this test, to check if the iterated process
files exactly match the files opened by the process.
If you mean further checking what exactly the file is, e.g. whether the
file is a pipe or a socket, then I can add that in the next version.
> 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