Re: [LTP] [PATCH] syscalls/unshare: New test: CLONE_NEWNS unshares fs info

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 844ae7a13..57338297a 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1725,6 +1725,7 @@ unlinkat01 unlinkat01
>  unshare01 unshare01
>  unshare02 unshare02
>  unshare03 unshare03
> +unshare04 unshare04
>  
>  #
>  # These tests require an unmounted block device
> diff --git a/testcases/kernel/syscalls/unshare/.gitignore b/testcases/kernel/syscalls/unshare/.gitignore
> index e5b5c261d..b1206e452 100644
> --- a/testcases/kernel/syscalls/unshare/.gitignore
> +++ b/testcases/kernel/syscalls/unshare/.gitignore
> @@ -1,3 +1,4 @@
>  /unshare01
>  /unshare02
>  /unshare03
> +/unshare04
> diff --git a/testcases/kernel/syscalls/unshare/unshare04.c b/testcases/kernel/syscalls/unshare/unshare04.c
> new file mode 100644
> index 000000000..0bbb9d19d
> --- /dev/null
> +++ b/testcases/kernel/syscalls/unshare/unshare04.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 lufei <lufei@uniontech.com>
> + */
> +
> +/*\
> + * This test case is to verify unshare(CLONE_NEWNS) also unshares filesystem
> + * information.
> + *
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include "tst_test.h"
> +#include "lapi/sched.h"
> +
> +#ifdef HAVE_UNSHARE

This shouldn't be needed anymore, the minimal glibc we support at the
moment is 2.22, unshare() if I'm looking right is included in glibc 2.4
and newer. I suppose that removing the configure check for unshare() is
long overdue.

> +#define TMP "/tmp"

The test shouldn't expect or touch directories outside the test
temporary directory. If you need a diretory to chdir() into create one
in the test setup() instead.

> +static void run(void)
> +{
> +	char *c_cwd;
> +	char *p_cwd;
> +	size_t size = 1024;
> +
> +	c_cwd = SAFE_MMAP(NULL, size, PROT_READ | PROT_WRITE,
> +			MAP_SHARED | MAP_ANONYMOUS,
> +			-1, 0);
> +
> +	p_cwd = SAFE_MALLOC(size);

These allocations should be made in test setup and freed in the test
cleanup.

> +	struct tst_clone_args args = {
> +		.flags = CLONE_FS,
> +		.exit_signal = SIGCHLD,
> +	};
> +
> +	if (!SAFE_CLONE(&args)) {
> +
> +		TST_EXP_PASS(unshare(CLONE_NEWNS));
> +
> +		SAFE_CHDIR(TMP);
> +		SAFE_GETCWD(c_cwd, size);
> +	} else {
> +		SAFE_WAIT(NULL);
> +
> +		SAFE_GETCWD(p_cwd, size);
> +
> +		tst_res(TDEBUG, "parent cwd: %s", p_cwd);
> +		tst_res(TDEBUG, "child cwd: %s", c_cwd);
> +
> +		//in parent, cwd stays in tst_tmpdir_path than TMP.
> +		if (strcmp(p_cwd, c_cwd) == 0)
> +			tst_res(TFAIL, "cwd not unshared as expected");
> +		else
> +			tst_res(TPASS, "cwd unshared");

We are validating that the CWD wasn't changed in parent after we have
done unshare() and chdir() in the child. Why do we even compare the
parent CWD againts the child? Rather than that we have to compare the
CWD in the parent againts the CWD at the start of the test.

What we should do is:

- do chdir() in child
- check in child that CWD was changed

- wait child in parent
- check in parent that CWD wasn't changed

For that all we have to do is to do getcwd() in the test setup and then
use it for a comparsion both in the child and parent.

> +	}
> +}
> +
> +static struct tst_test test = {
> +	.forks_child = 1,
> +	.needs_root = 1,
> +	.needs_tmpdir = 1,
> +	.test_all = run,
> +};
> +
> +#else
> +TST_TEST_TCONF("unshare syscall is undefined.");
> +#endif
> -- 
> 2.39.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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