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:
 <AM6PR03MB5080EB3C342F9114ABB5B4FA993D2@AM6PR03MB5080.eurprd03.prod.outlook.com>
Date: Tue, 10 Dec 2024 14:17:39 +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/27 11:21, Jiri Olsa wrote:
> 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

Sorry for the delay, I have been a bit busy recently.

Thanks for letting me know, fentry is a good approach.

I used fentry in patch series v5.