linux-kernel - Re: [PATCH bpf-next v4 2/5] selftests/bpf: Add tests for open-coded style process file iterator

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z0cAz3uOGRcl36Eu@krava>
Date: Wed, 27 Nov 2024 12:21:51 +0100
From: Jiri Olsa <olsajiri@...il.com>
To: Juntong Deng <juntong.deng@...look.com>
Cc: Jiri Olsa <olsajiri@...il.com>, 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 26, 2024 at 10:24:07PM +0000, Juntong Deng wrote:
> 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.

I see, I missed the close_range call.. anyway I'd still prefer pipe to busy waiting

perhaps you could use fentry probe triggered by the task_file_test_process
and do the fd/file iteration in there? that way there'be no need for the sync

jirka