Re: [LTP] [PATCH v2 2/5] fanotify21: Stop relying on exited child for pidfd error

[Petr Vorel <pvorel@suse.cz>]

Hi all,

> fanotify21 used to verify the pidfd error case by generating an event
> from a child process and reading the event after the child had exited.
> This is no longer a stable way to trigger a pidfd error, because newer
> kernels can report pidfds for exited event processes.

> Read the event from a child process in a descendant PID namespace
> instead. The reader cannot get the pidfd in the parent PID namespace
> from fanotify, so pidfd creation will fail and the test can verify
> FAN_NOPIDFD, or -ESRCH with FAN_REPORT_FD_ERROR.

> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

...
>  	/*
> -	 * Generate the event in either self or a child process. Event
> -	 * generation in a child process is done so that the FAN_NOPIDFD case
> -	 * can be verified.
> +	 * Read the event in either self or a child process in a descendant
> +	 * PID namespace. A reader in a descendant PID namespace cannot obtain
> +	 * the pidfd of the event process in the parent PID namespace so that
> +	 * the FAN_NOPIDFD case can be verified.
>  	 */
> -	if (tc->fork)
> -		do_fork();
> -	else
> -		generate_event();
> +	if (tc->want_pidfd_err && (reader_pid = SAFE_CLONE(&clone_args))) {
nit: although I'd write it this way as well, checkpatch does not like this
effective way :) (do not use assignment in if condition).

> +		SAFE_WAITPID(reader_pid, &reader_exit_status, 0);
> +		if (!WIFEXITED(reader_exit_status) || WEXITSTATUS(reader_exit_status))
> +			tst_brk(TBROK, "reader process exited incorrectly");

nit: And the above should be probably replaced with tst_reap_children().

But because I should have merged this long time ago to prevent false positive on
7.2, merging it now. Cleanup can be done later (together with using designated
initializers).

Merged the rest of the patch, thank you!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp