Re: [PATCH 5/5] fanotify21: Add FAN_REPORT_TID pidfd coverage

On Tue 16-06-26 02:06:29, AnonymeMeow wrote:
> Add a test case that generates an event from a thread. Without
> FAN_REPORT_TID, the reported pidfd is expected to refer to the thread
> group leader. With FAN_REPORT_TID, it is expected to refer to the
> triggering thread. Keep the event thread alive until the reported pidfd
> is verified, because pidfd fdinfo reports Pid: -1 after the thread
> exits.
> 
> Signed-off-by: AnonymeMeow <[email protected]>

Nice! Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify21.c     | 47 ++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
> index fd43c8c1c..8a5b35334 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify21.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
> @@ -24,6 +24,7 @@
>  #include "tst_test.h"
>  #include "tst_safe_stdio.h"
>  #include "tst_safe_macros.h"
> +#include "tst_safe_pthread.h"
>  #include "lapi/pidfd.h"
>  
>  #ifdef HAVE_SYS_FANOTIFY_H
> @@ -46,6 +47,7 @@ struct pidfd_fdinfo_t {
>  
>  static struct test_case_t {
>  	char *name;
> +	int event_by_thread;
>  	int want_pidfd_err;
>  	int remount_ro;
>  } test_cases[] = {
> @@ -53,15 +55,24 @@ static struct test_case_t {
>  		"return a valid pidfd for event created by self",
>  		0,
>  		0,
> +		0,
> +	},
> +	{
> +		"return a valid pidfd for event created by thread",
> +		1,
> +		0,
> +		0,
>  	},
>  	{
>  		"return invalid pidfd for reader in descendant PID namespace",
> +		0,
>  		1,
>  		0,
>  	},
>  	{
>  		"fail to open rw fd for event created on read-only mount",
>  		0,
> +		0,
>  		1,
>  	},
>  };
> @@ -107,6 +118,26 @@ static void generate_event(void)
>  	SAFE_CLOSE(fd);
>  }
>  
> +static void *generate_event_pthread(void *args LTP_ATTRIBUTE_UNUSED)
> +{
> +	generate_event();
> +	TST_CHECKPOINT_WAIT(0);
> +	return NULL;
> +}
> +
> +static pthread_t event_thread_create(void)
> +{
> +	pthread_t pthread_id;
> +	SAFE_PTHREAD_CREATE(&pthread_id, NULL, generate_event_pthread, NULL);
> +	return pthread_id;
> +}
> +
> +static void event_thread_join(pthread_t pthread_id)
> +{
> +	TST_CHECKPOINT_WAKE(0);
> +	SAFE_PTHREAD_JOIN(pthread_id, NULL);
> +}
> +
>  static const char *test_variant_name(void)
>  {
>  	switch (tst_variant) {
> @@ -164,6 +195,7 @@ static void do_test(unsigned int num)
>  	int nopidfd_err = tc->want_pidfd_err ?
>  			  (TST_VARIANT_FD_ERROR ? -ESRCH : FAN_NOPIDFD) : 0;
>  	int fd_err = (tc->remount_ro && TST_VARIANT_FD_ERROR) ? -EROFS : 0;
> +	pthread_t event_thread;
>  	pid_t reader_pid;
>  	int reader_exit_status;
>  
> @@ -185,7 +217,16 @@ static void do_test(unsigned int num)
>  	SAFE_MOUNT("none", MOUNT_PATH, "none", MS_BIND | MS_REMOUNT |
>  		   (tc->remount_ro ? MS_RDONLY : 0), NULL);
>  
> -	generate_event();
> +	/*
> +	 * Generate the event by either self or a thread. Event generation in
> +	 * a thread is done so that in FAN_REPORT_TID mode the pidfd can be
> +	 * verified to refer to the tid, not the tgid. The thread waits until
> +	 * the pidfd is verified before exiting.
> +	 */
> +	if (tc->event_by_thread)
> +		event_thread = event_thread_create();
> +	else
> +		generate_event();
>  
>  	/*
>  	 * Read the event in either self or a child process in a descendant
> @@ -348,6 +389,9 @@ next_event:
>  
>  	if (tc->want_pidfd_err)
>  		exit(0);
> +
> +	if (tc->event_by_thread)
> +		event_thread_join(event_thread);
>  }
>  
>  static void do_cleanup(void)
> @@ -370,6 +414,7 @@ static struct tst_test test = {
>  	.mount_device = 1,
>  	.mntpoint = MOUNT_PATH,
>  	.forks_child = 1,
> +	.needs_checkpoints = 1,
>  };
>  
>  #else
> -- 
> 2.54.0
> 
-- 
Jan Kara <jack-IBi9RG/[email protected]>
SUSE Labs, CR

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