Re: [LTP] [PATCH v2 4/8] Add ioctl_pidfd02 test

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore
> index aa952c1a7bae0ae2dbb04de0595f10d508b6759a..1c81c2bed8db952af0c93fb1ce5bfbad82794b60 100644
> --- a/testcases/kernel/syscalls/ioctl/.gitignore
> +++ b/testcases/kernel/syscalls/ioctl/.gitignore
> @@ -30,3 +30,4 @@
>  /ioctl_ficlonerange02
>  /ioctl_fiemap01
>  /ioctl_pidfd01
> +/ioctl_pidfd02
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..a5a9a561ff676607d68a33ed5572d4c3cdb28f26
> --- /dev/null
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_pidfd02.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * Check if the ioctl() function allows retrieval of a child's exit code
> + * using PIDFD_INFO_EXIT from a process that can be isolated or not from the
> + * child.
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/pidfd.h"
> +#include "lapi/sched.h"
> +
> +static struct tst_clone_args *args;
> +static struct pidfd_info *info0, *info1;
> +
> +static void run(unsigned int isolate)
> +{
> +	int status;
> +	int pidfd = 0;
> +	pid_t pid_child;
> +
> +	memset(args, 0, sizeof(struct tst_clone_args));
> +	memset(info0, 0, sizeof(struct pidfd_info));
> +	memset(info1, 0, sizeof(struct pidfd_info));
> +
> +	if (isolate) {
> +		args->flags = CLONE_PIDFD | CLONE_NEWUSER | CLONE_NEWPID;
> +		args->pidfd = (uint64_t)&pidfd;
> +		args->exit_signal = SIGCHLD;
> +
> +		pid_child = SAFE_CLONE(args);
> +	} else {
> +		pid_child = SAFE_FORK();
> +	}
> +
> +	if (!pid_child) {
> +		TST_CHECKPOINT_WAIT(0);
> +		exit(100);
> +	}
> +
> +	if (!isolate)
> +		pidfd = SAFE_PIDFD_OPEN(pid_child, 0);
> +
> +	/* child is not reaped and ioctl() won't provide any exit status info */
> +	info0->mask = PIDFD_INFO_EXIT;
> +	SAFE_IOCTL(pidfd, PIDFD_GET_INFO, info0);
> +	TST_EXP_EQ_LI(info0->mask & PIDFD_INFO_EXIT, 0);
> +	TST_EXP_EQ_LI(info0->exit_code, 0);
> +
> +	TST_CHECKPOINT_WAKE(0);
> +
> +	SAFE_WAITPID(pid_child, &status, 0);
> +
> +	/* child is now reaped and ioctl() will provide the exit status */
> +	info1->mask = PIDFD_INFO_EXIT;
> +	SAFE_IOCTL(pidfd, PIDFD_GET_INFO, info1);
> +
> +	TST_EXP_EQ_LI(info1->mask & PIDFD_INFO_EXIT, PIDFD_INFO_EXIT);
> +	TST_EXP_EQ_LI(WIFEXITED(info1->exit_code), WIFEXITED(status));
> +	TST_EXP_EQ_LI(WEXITSTATUS(info1->exit_code), WEXITSTATUS(status));
> +
> +	TST_EXP_EXPR(info0->exit_code != info1->exit_code,
> +	      "info0->exit_code (%i) != info1->exit_code (%i)",
> +	      info0->exit_code, info1->exit_code);

Wouldn't it be just easier to check that info1->exit_code == 100? We
already asserted that info0->exit_code == 0 and because 100 != 0 we will
get this assertion already covered.

> +}
> +
> +static struct tst_test test = {
> +	.test = run,
> +	.tcnt = 2,
> +	.forks_child = 1,
> +	.needs_checkpoints = 1,
> +	.min_kver = "6.15",

We just backported this into 6.12 in SUSE so we would need a different
runtime detection than this. Generally it's a good idea to avoid kernel
version checks if possible.

Looking at the kernel code it looks that if the PIDFD_INFO_EXIT is not
supported we will get ESRCH in the second case. So I suppose that better
fix would be to allow ESRCH in the second case if kernel is older than
6.15 instead of skipping the whole test.

> +	.bufs = (struct tst_buffers []) {
> +		{&args, .size = sizeof(*args)},
> +		{&info0, .size = sizeof(*info0)},
> +		{&info1, .size = sizeof(*info1)},
> +		{}
> +	}
> +};
> 
> -- 
> 2.50.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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