Re: [LTP] [PATCH v4] unshare03.c: Add test coverage for dup_fd() failure handling in unshare_fd()

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi,

thanks for editing the last version. A couple of comments below.

On 3/4/25 05:06, Wei Gao via ltp wrote:
> Add a test case based on kernel self-test unshare_test.c to check that
> the kernel handles the EMFILE error when a parent process changes file
> descriptor limits and the child process tries to unshare (CLONE_FILES).
>
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>   runtest/syscalls                              |  1 +
>   testcases/kernel/syscalls/unshare/.gitignore  |  1 +
>   testcases/kernel/syscalls/unshare/unshare03.c | 77 +++++++++++++++++++
>   3 files changed, 79 insertions(+)
>   create mode 100644 testcases/kernel/syscalls/unshare/unshare03.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index ded035ee8..10800c1a3 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1715,6 +1715,7 @@ unlinkat01 unlinkat01
>   
>   unshare01 unshare01
>   unshare02 unshare02
> +unshare03 unshare03
>   
>   #
>   # These tests require an unmounted block device
> diff --git a/testcases/kernel/syscalls/unshare/.gitignore b/testcases/kernel/syscalls/unshare/.gitignore
> index 855ffd055..e5b5c261d 100644
> --- a/testcases/kernel/syscalls/unshare/.gitignore
> +++ b/testcases/kernel/syscalls/unshare/.gitignore
> @@ -1,2 +1,3 @@
>   /unshare01
>   /unshare02
> +/unshare03
> diff --git a/testcases/kernel/syscalls/unshare/unshare03.c b/testcases/kernel/syscalls/unshare/unshare03.c
> new file mode 100644
> index 000000000..c1f13bcd5
> --- /dev/null
> +++ b/testcases/kernel/syscalls/unshare/unshare03.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 Al Viro <viro@zeniv.linux.org.uk>
> + * Copyright (C) 2024 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * This test case based on kernel self-test unshare_test.c to check that
> + * the kernel handles the EMFILE error when a parent process changes file
> + * descriptor limits and the child process tries to unshare (CLONE_FILES).
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include "tst_test.h"
> +#include "config.h"
> +#include "lapi/sched.h"
> +
> +#define FS_NR_OPEN "/proc/sys/fs/nr_open"
> +
> +#ifdef HAVE_UNSHARE
> +
> +static void run(void)
> +{
> +	int nr_open;
> +	struct rlimit rlimit;
> +	struct tst_clone_args args = {
> +		.flags = CLONE_FILES,
> +		.exit_signal = SIGCHLD,
> +	};
> +
> +	SAFE_FILE_SCANF(FS_NR_OPEN, "%d", &nr_open);
> +	tst_res(TDEBUG, "Maximum number of file descriptors: %d", nr_open);
> +
> +	SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_open + 1024);
> +
> +	SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit);
> +
> +	rlimit.rlim_cur = nr_open + 1024;
> +	rlimit.rlim_max = nr_open + 1024;
> +
> +	SAFE_SETRLIMIT(RLIMIT_NOFILE, &rlimit);
> +	tst_res(TDEBUG, "Set new maximum number of file descriptors to : %d",
> +		nr_open + 1024);
> +
> +	SAFE_DUP2(2, nr_open + 64);
> +
> +	if (!SAFE_CLONE(&args)) {
> +		SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_open);
> +		TST_EXP_FAIL(unshare(CLONE_FILES), EMFILE);
> +		TST_CHECKPOINT_WAKE(0);
There's no need to use synchronization mechanisms because at the end of 
the test we call tst_reap_children() waiting for all the children to be 
completed.
> +		exit(0);
> +	}
> +
> +	TST_CHECKPOINT_WAIT(0);
> +}
> +
> +static void setup(void)
> +{
> +	clone3_supported_by_kernel();
> +}
> +
> +static struct tst_test test = {
> +	.forks_child = 1,
> +	.needs_root = 1,
> +	.test_all = run,
> +	.setup = setup,
> +	.needs_checkpoints = 1,
> +	.save_restore = (const struct tst_path_val[]) {
> +		{FS_NR_OPEN, NULL, TST_SR_TCONF},
> +		{}
> +	},
> +};
> +
> +#else
> +TST_TEST_TCONF("unshare syscall is undefined.");
> +#endif

The rest looks good. If you want I can edit the checkpoint thing and 
merge this patch.

Kind regards,
Andrea Cervesato


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