Re: [LTP] [PATCH STAGING v4 12/16] fchroot09: test setns escape from failfs root

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Verify that entering failfs with fchroot() is hard to undo: a process
> inside counts as chrooted, so the remaining way out is a pre-opened
> mount namespace fd. setns() into it resets both the root and the
> working directory.
> 
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  runtest/staging                               |  1 +
>  testcases/kernel/syscalls/fchroot/.gitignore  |  1 +
>  testcases/kernel/syscalls/fchroot/fchroot09.c | 86 +++++++++++++++++++++++++++
>  3 files changed, 88 insertions(+)
> 
> diff --git a/runtest/staging b/runtest/staging
> index 8dfe95c4a..23f6c6a20 100644
> --- a/runtest/staging
> +++ b/runtest/staging
> @@ -8,3 +8,4 @@ fchroot05 fchroot05
>  fchroot06 fchroot06
>  fchroot07 fchroot07
>  fchroot08 fchroot08
> +fchroot09 fchroot09
> diff --git a/testcases/kernel/syscalls/fchroot/.gitignore b/testcases/kernel/syscalls/fchroot/.gitignore
> index 0a2510d8a..b577da61f 100644
> --- a/testcases/kernel/syscalls/fchroot/.gitignore
> +++ b/testcases/kernel/syscalls/fchroot/.gitignore
> @@ -7,3 +7,4 @@ fchroot06
>  fchroot07
>  fchroot07_child
>  fchroot08
> +fchroot09
> diff --git a/testcases/kernel/syscalls/fchroot/fchroot09.c b/testcases/kernel/syscalls/fchroot/fchroot09.c
> new file mode 100644
> index 000000000..bc9324ceb
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fchroot/fchroot09.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * Test that :manpage:`setns(2)` escapes the failfs root.
> + *
> + * Entering failfs with :manpage:`fchroot(2)` is hard to undo: a process
> + * inside counts as chrooted, so :manpage:`chroot(2)` and :manpage:`fchroot(2)`
> + * back out require ``CAP_SYS_CHROOT``. The remaining way out is a pre-opened
> + * mount namespace file descriptor: :manpage:`setns(2)` into it resets both the
> + * root and the working directory.
> + *
> + * Root is required because entering failfs requires ``CAP_SYS_CHROOT`` and
> + * :manpage:`setns(2)` into the mount namespace requires ``CAP_SYS_ADMIN``.
> + *
> + * The test runs in a forked child so the root of the parent process is
> + * left untouched.
> + */
> +
> +#define _GNU_SOURCE
> +#include <fcntl.h>
> +#include <sys/stat.h>
> +#include "tst_test.h"
> +#include "lapi/fcntl.h"
> +#include "lapi/sched.h"
> +#include "lapi/setns.h"
> +#include "lapi/syscalls.h"
> +
> +#define ABSDIR "abs"
> +
> +static void run(void)
> +{
> +	if (SAFE_FORK())
> +		return;
> +
> +	struct stat realroot, st;
> +	int nsfd;
> +
> +	SAFE_STAT("/", &realroot);
> +	nsfd = SAFE_OPEN("/proc/self/ns/mnt", O_RDONLY);
> +
> +	TST_EXP_PASS(tst_syscall(__NR_fchroot, FD_FAILFS_ROOT, 0),
> +		"fchroot() with the FD_FAILFS_ROOT sentinel");
> +
> +	TST_EXP_FAIL2(open(ABSDIR, O_PATH), EOPNOTSUPP,
> +		"absolute lookup after entering failfs");
> +
> +	TST_EXP_PASS(setns(nsfd, CLONE_NEWNS),
> +		"setns() back into the mount namespace");
> +
> +	SAFE_CLOSE(nsfd);
> +
> +	SAFE_STAT("/", &st);
> +	TST_EXP_EXPR(st.st_dev == realroot.st_dev &&
> +		st.st_ino == realroot.st_ino,
> +		"root restored after setns()");
> +
> +	SAFE_STAT(".", &st);
> +	TST_EXP_EXPR(st.st_dev == realroot.st_dev &&
> +		st.st_ino == realroot.st_ino,
> +		"working directory restored after setns()");
> +
> +	exit(0);

Here as well, no need for the exit.

> +}
> +
> +static void setup(void)
> +{
> +	char *tmpdir;
> +	char abs_path[PATH_MAX];
> +
> +	tmpdir = tst_tmpdir_path();
> +	snprintf(abs_path, sizeof(abs_path), "%s/%s", tmpdir, "target");
> +
> +	SAFE_TOUCH("target", 0644, NULL);
> +	SAFE_SYMLINK(abs_path, ABSDIR);

Here as well, why are all the absolute directories symlinks? It makes
sense to have a test symlink to absolute directory restarts the lookup
and fails, but why don't we use just abs_path in most of the tests?

I suppose that we can even open just "/" with O_PATH to check if the
failfs root is active or not...

> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.needs_tmpdir = 1,
> +};
> 
> -- 
> 2.51.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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