All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Zorro Lang <zlang@redhat.com>, fstests <fstests@vger.kernel.org>
Cc: Christian Brauner <brauner@kernel.org>,
	Dave Chinner <david@fromorbit.com>,
	Eryu Guan <guaneryu@gmail.com>,
	Amir Goldstein <amir73il@gmail.com>,
	Christoph Hellwig <hch@lst.de>,
	"Darrick J. Wong" <djwong@kernel.org>
Subject: [PATCH v2 13/13] vfs/idmapped-mounts: remove invalid test
Date: Thu, 12 May 2022 18:52:50 +0200	[thread overview]
Message-ID: <20220512165250.450989-14-brauner@kernel.org> (raw)
In-Reply-To: <20220512165250.450989-1-brauner@kernel.org>

The threaded mount interaction test validates that the idmapping of a
mount behaves correctly when changed while another process already has a
writable file descriptor to that mount. In newer kernels it isn't
possible to change a mount's idmapped if someone has a writable file
descriptor to that mount.

Link: e1bbcd277a53 ("fs: hold writers when changing mount's idmapping")
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 src/vfs/idmapped-mounts.c | 241 --------------------------------------
 src/vfs/vfstest.c         |   1 -
 2 files changed, 242 deletions(-)

diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
index d3763c2d..63297d5f 100644
--- a/src/vfs/idmapped-mounts.c
+++ b/src/vfs/idmapped-mounts.c
@@ -6438,246 +6438,6 @@ out:
 	return fret;
 }
 
-#define PTR_TO_INT(p) ((int)((intptr_t)(p)))
-#define INT_TO_PTR(u) ((void *)((intptr_t)(u)))
-
-struct threaded_args {
-	const struct vfstest_info *info;
-	int open_tree_fd;
-};
-
-static void *idmapped_mount_create_cb(void *data)
-{
-	int fret = EXIT_FAILURE;
-	struct mount_attr attr = {
-		.attr_set = MOUNT_ATTR_IDMAP,
-	};
-	struct threaded_args *args = data;
-
-	/* Changing mount properties on a detached mount. */
-	attr.userns_fd	= get_userns_fd(0, 10000, 10000);
-	if (attr.userns_fd < 0) {
-		log_stderr("failure: get_userns_fd");
-		goto out;
-	}
-
-	if (sys_mount_setattr(args->open_tree_fd, "", AT_EMPTY_PATH, &attr, sizeof(attr))) {
-		log_stderr("failure: sys_mount_setattr");
-		goto out;
-	}
-
-	fret = EXIT_SUCCESS;
-
-out:
-	safe_close(attr.userns_fd);
-	pthread_exit(INT_TO_PTR(fret));
-}
-
-/* This tries to verify that we never see an inconistent ownership on-disk and
- * can't write invalid ids to disk. To do this we create a race between
- * idmapping a mount and creating files on it.
- * Note, while it is perfectly fine to see overflowuid and overflowgid as owner
- * if we create files through the open_tree_fd before the mount is idmapped but
- * look at the files after the mount has been idmapped in this test it can never
- * be the case that we see overflowuid and overflowgid when we access the file
- * through a non-idmapped mount (in the initial user namespace).
- */
-static void *idmapped_mount_operations_cb(void *data)
-{
-	int file1_fd = -EBADF, file2_fd = -EBADF, dir1_fd = -EBADF,
-	    dir1_fd2 = -EBADF, fret = EXIT_FAILURE;
-	struct threaded_args *args = data;
-	const struct vfstest_info *info = args->info;
-
-	if (!switch_fsids(10000, 10000)) {
-		log_stderr("failure: switch fsids");
-		goto out;
-	}
-
-	file1_fd = openat(args->open_tree_fd, FILE1,
-			  O_CREAT | O_EXCL | O_CLOEXEC, 0644);
-	if (file1_fd < 0) {
-		log_stderr("failure: openat");
-		goto out;
-	}
-
-	file2_fd = openat(args->open_tree_fd, FILE2,
-			  O_CREAT | O_EXCL | O_CLOEXEC, 0644);
-	if (file2_fd < 0) {
-		log_stderr("failure: openat");
-		goto out;
-	}
-
-	if (mkdirat(args->open_tree_fd, DIR1, 0777)) {
-		log_stderr("failure: mkdirat");
-		goto out;
-	}
-
-	dir1_fd = openat(args->open_tree_fd, DIR1,
-			 O_RDONLY | O_DIRECTORY | O_CLOEXEC);
-	if (dir1_fd < 0) {
-		log_stderr("failure: openat");
-		goto out;
-	}
-
-	if (!__expected_uid_gid(args->open_tree_fd, FILE1, 0, 0, 0, false) &&
-	    !__expected_uid_gid(args->open_tree_fd, FILE1, 0, 10000, 10000, false) &&
-	    !__expected_uid_gid(args->open_tree_fd, FILE1, 0, info->t_overflowuid, info->t_overflowgid, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	if (!__expected_uid_gid(args->open_tree_fd, FILE2, 0, 0, 0, false) &&
-	    !__expected_uid_gid(args->open_tree_fd, FILE2, 0, 10000, 10000, false) &&
-	    !__expected_uid_gid(args->open_tree_fd, FILE2, 0, info->t_overflowuid, info->t_overflowgid, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	if (!__expected_uid_gid(args->open_tree_fd, DIR1, 0, 0, 0, false) &&
-	    !__expected_uid_gid(args->open_tree_fd, DIR1, 0, 10000, 10000, false) &&
-	    !__expected_uid_gid(args->open_tree_fd, DIR1, 0, info->t_overflowuid, info->t_overflowgid, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	if (!__expected_uid_gid(dir1_fd, "", AT_EMPTY_PATH, 0, 0, false) &&
-	    !__expected_uid_gid(dir1_fd, "", AT_EMPTY_PATH, 10000, 10000, false) &&
-	    !__expected_uid_gid(dir1_fd, "", AT_EMPTY_PATH, info->t_overflowuid, info->t_overflowgid, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	dir1_fd2 = openat(info->t_dir1_fd, DIR1,
-			 O_RDONLY | O_DIRECTORY | O_CLOEXEC);
-	if (dir1_fd2 < 0) {
-		log_stderr("failure: openat");
-		goto out;
-	}
-
-        if (!__expected_uid_gid(info->t_dir1_fd, FILE1, 0, 0, 0, false) &&
-	    !__expected_uid_gid(info->t_dir1_fd, FILE1, 0, 10000, 10000, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	if (!__expected_uid_gid(info->t_dir1_fd, FILE2, 0, 0, 0, false) &&
-	    !__expected_uid_gid(info->t_dir1_fd, FILE2, 0, 10000, 10000, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	if (!__expected_uid_gid(info->t_dir1_fd, DIR1, 0, 0, 0, false) &&
-	    !__expected_uid_gid(info->t_dir1_fd, DIR1, 0, 10000, 10000, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	if (!__expected_uid_gid(info->t_dir1_fd, DIR1, 0, 0, 0, false) &&
-	    !__expected_uid_gid(info->t_dir1_fd, DIR1, 0, 10000, 10000, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	if (!__expected_uid_gid(dir1_fd2, "", AT_EMPTY_PATH, 0, 0, false) &&
-	    !__expected_uid_gid(dir1_fd2, "", AT_EMPTY_PATH, 10000, 10000, false)) {
-		log_stderr("failure: expected_uid_gid");
-		goto out;
-	}
-
-	fret = EXIT_SUCCESS;
-
-out:
-	safe_close(file1_fd);
-	safe_close(file2_fd);
-	safe_close(dir1_fd);
-	safe_close(dir1_fd2);
-
-	pthread_exit(INT_TO_PTR(fret));
-}
-
-static int threaded_idmapped_mount_interactions(const struct vfstest_info *info)
-{
-	int i;
-	int fret = -1;
-	pid_t pid;
-	pthread_attr_t thread_attr;
-	pthread_t threads[2];
-
-	pthread_attr_init(&thread_attr);
-
-	for (i = 0; i < 1000; i++) {
-		int ret1 = 0, ret2 = 0, tret1 = 0, tret2 = 0;
-
-		pid = fork();
-		if (pid < 0) {
-			log_stderr("failure: fork");
-			goto out;
-		}
-		if (pid == 0) {
-			int open_tree_fd = -EBADF;
-			struct threaded_args args = {
-				.info = info,
-				.open_tree_fd = -EBADF,
-			};
-
-			open_tree_fd = sys_open_tree(info->t_dir1_fd, "",
-						     AT_EMPTY_PATH |
-						     AT_NO_AUTOMOUNT |
-						     AT_SYMLINK_NOFOLLOW |
-						     OPEN_TREE_CLOEXEC |
-						     OPEN_TREE_CLONE);
-			if (open_tree_fd < 0)
-				die("failure: sys_open_tree");
-
-			args.open_tree_fd = open_tree_fd;
-
-			if (pthread_create(&threads[0], &thread_attr,
-					   idmapped_mount_create_cb,
-					   &args))
-				die("failure: pthread_create");
-
-			if (pthread_create(&threads[1], &thread_attr,
-					   idmapped_mount_operations_cb,
-					   &args))
-				die("failure: pthread_create");
-
-			ret1 = pthread_join(threads[0], INT_TO_PTR(tret1));
-			ret2 = pthread_join(threads[1], INT_TO_PTR(tret2));
-
-			if (ret1) {
-				errno = ret1;
-				die("failure: pthread_join");
-			}
-
-			if (ret2) {
-				errno = ret2;
-				die("failure: pthread_join");
-			}
-
-			if (tret1 || tret2)
-				exit(EXIT_FAILURE);
-
-			exit(EXIT_SUCCESS);
-
-		}
-
-		if (wait_for_pid(pid)) {
-			log_stderr("failure: iteration %d", i);
-			goto out;
-		}
-
-		rm_r(info->t_dir1_fd, ".");
-
-	}
-
-	fret = 0;
-	log_debug("Ran test");
-
-out:
-	return fret;
-}
-
 static int nested_userns(const struct vfstest_info *info)
 {
 	int fret = -1;
@@ -7941,7 +7701,6 @@ static const struct test_struct t_idmapped_mounts[] = {
 	{ sticky_bit_rename_idmapped_mounts_in_userns,			true,	"sticky bit rename operations on idmapped mounts in user namespace",				},
 	{ symlink_idmapped_mounts,					true,	"symlink from idmapped mounts",									},
 	{ symlink_idmapped_mounts_in_userns,				true,	"symlink from idmapped mounts in user namespace",						},
-	{ threaded_idmapped_mount_interactions,				true,	"threaded operations on idmapped mounts",							},
 };
 
 const struct test_suite s_idmapped_mounts = {
diff --git a/src/vfs/vfstest.c b/src/vfs/vfstest.c
index 67aa3bda..29ac0bec 100644
--- a/src/vfs/vfstest.c
+++ b/src/vfs/vfstest.c
@@ -13,7 +13,6 @@
 #include <limits.h>
 #include <linux/limits.h>
 #include <linux/types.h>
-#include <pthread.h>
 #include <pwd.h>
 #include <sched.h>
 #include <stdbool.h>
-- 
2.34.1


  parent reply	other threads:[~2022-05-12 16:53 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-12 16:52 [PATCH v2 00/13] rename & split tests Christian Brauner
2022-05-12 16:52 ` [PATCH v2 01/13] idmapped-mounts: make all tests set their required feature flags Christian Brauner
2022-05-16  5:50   ` Christoph Hellwig
2022-05-12 16:52 ` [PATCH v2 02/13] src: rename idmapped-mounts folder Christian Brauner
2022-05-12 16:52 ` [PATCH v2 03/13] src/vfs: rename idmapped-mounts.c file Christian Brauner
2022-05-12 16:52 ` [PATCH v2 04/13] vfstest: rename struct t_idmapped_mounts Christian Brauner
2022-05-12 16:52 ` [PATCH v2 05/13] utils: add missing global.h include Christian Brauner
2022-05-12 16:52 ` [PATCH v2 07/13] utils: move helpers into utils Christian Brauner
2022-05-12 16:52 ` [PATCH v2 08/13] missing: move sys_execveat() to missing.h Christian Brauner
2022-05-12 16:52 ` [PATCH v2 09/13] utils: add struct test_suite Christian Brauner
2022-05-12 16:52 ` [PATCH v2 12/13] vfstest: split out remaining idmapped mount tests Christian Brauner
2022-05-12 16:52 ` Christian Brauner [this message]
2022-05-16  5:53   ` [PATCH v2 13/13] vfs/idmapped-mounts: remove invalid test Christoph Hellwig
2022-05-12 20:19 ` [PATCH v2 00/13] rename & split tests Zorro Lang
2022-05-14 17:59 ` Zorro Lang
2022-05-16  8:02   ` Christian Brauner
2022-05-21 23:13 ` Dave Chinner
2022-05-22  1:07   ` Jens Axboe
2022-05-22  2:19     ` Jens Axboe
2022-05-23  0:13       ` Dave Chinner
2022-05-23  0:57         ` Jens Axboe
2022-05-23 10:41           ` Dave Chinner
2022-05-23 12:28             ` Jens Axboe
2022-05-23  9:44   ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220512165250.450989-14-brauner@kernel.org \
    --to=brauner@kernel.org \
    --cc=amir73il@gmail.com \
    --cc=david@fromorbit.com \
    --cc=djwong@kernel.org \
    --cc=fstests@vger.kernel.org \
    --cc=guaneryu@gmail.com \
    --cc=hch@lst.de \
    --cc=zlang@redhat.com \
    --subject='Re: [PATCH v2 13/13] vfs/idmapped-mounts: remove invalid test' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.