All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper
@ 2022-01-13 13:24 Christian Brauner
  2022-01-13 13:24 ` [PATCH 2/2] idmapped-mounts: always run generic vfs tests Christian Brauner
  2022-01-16  4:51 ` [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper Eryu Guan
  0 siblings, 2 replies; 7+ messages in thread
From: Christian Brauner @ 2022-01-13 13:24 UTC (permalink / raw)
  To: fstests, Eryu Guan
  Cc: Christoph Hellwig, Seth Forshee, Christian Brauner, Eryu Guan

Move the check whether the underlying filesystem supports idmapped
mounts into a separate helper. We will use it in the following patch to
make it possible to always run all tests that don't require idmapped
mounts.

Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Eryu Guan <guaneryu@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: fstests@vger.kernel.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 src/idmapped-mounts/idmapped-mounts.c | 57 ++++++++++++++-------------
 1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index da690779..a78a901f 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -13907,6 +13907,35 @@ static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
 	return true;
 }
 
+static bool fs_allow_idmap(void)
+{
+	int ret;
+	int open_tree_fd = -EBADF;
+	struct mount_attr attr = {
+		.attr_set	= MOUNT_ATTR_IDMAP,
+		.userns_fd	= -EBADF,
+	};
+
+	/* Changing mount properties on a detached mount. */
+	attr.userns_fd = get_userns_fd(0, 1000, 1);
+	if (attr.userns_fd < 0)
+		exit(EXIT_FAILURE);
+
+	open_tree_fd = sys_open_tree(t_mnt_fd, "",
+				     AT_EMPTY_PATH | AT_NO_AUTOMOUNT |
+				     AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC |
+				     OPEN_TREE_CLONE);
+	if (open_tree_fd < 0)
+		ret = -1;
+	else
+		ret = sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr,
+					sizeof(attr));
+	close(open_tree_fd);
+	close(attr.userns_fd);
+
+	return ret == 0;
+}
+
 int main(int argc, char *argv[])
 {
 	int fret, ret;
@@ -13987,34 +14016,8 @@ int main(int argc, char *argv[])
 	 * idmapped mounts.
 	 */
 	if (supported) {
-		int open_tree_fd = -EBADF;
-		struct mount_attr attr = {
-			.attr_set	= MOUNT_ATTR_IDMAP,
-			.userns_fd	= -EBADF,
-		};
-
-		/* Changing mount properties on a detached mount. */
-		attr.userns_fd	= get_userns_fd(0, 1000, 1);
-		if (attr.userns_fd < 0)
+		if (!fs_allow_idmap())
 			exit(EXIT_FAILURE);
-
-		open_tree_fd = sys_open_tree(t_mnt_fd, "",
-					     AT_EMPTY_PATH |
-					     AT_NO_AUTOMOUNT |
-					     AT_SYMLINK_NOFOLLOW |
-					     OPEN_TREE_CLOEXEC |
-					     OPEN_TREE_CLONE);
-		if (open_tree_fd < 0)
-			ret = -1;
-		else
-			ret = sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr, sizeof(attr));
-
-		close(open_tree_fd);
-		close(attr.userns_fd);
-
-		if (ret)
-			exit(EXIT_FAILURE);
-
 		exit(EXIT_SUCCESS);
 	}
 

base-commit: c61c1e2f4cad89d4f900eaef173616b309228110
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] idmapped-mounts: always run generic vfs tests
  2022-01-13 13:24 [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper Christian Brauner
@ 2022-01-13 13:24 ` Christian Brauner
  2022-01-16  4:52   ` Eryu Guan
  2022-01-16  4:51 ` [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper Eryu Guan
  1 sibling, 1 reply; 7+ messages in thread
From: Christian Brauner @ 2022-01-13 13:24 UTC (permalink / raw)
  To: fstests, Eryu Guan
  Cc: Christoph Hellwig, Seth Forshee, Christian Brauner, Eryu Guan

Make it possible to always run all the tests in the testsuite that don't
require idmapped mounts. Now all filesystems can benefit from the
generic vfs tests that we currently implement. This means setgid
inheritance and other tests will be run for all filesystems not matter
if they support idmapped mounts or not.

Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Eryu Guan <guaneryu@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: fstests@vger.kernel.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 src/idmapped-mounts/idmapped-mounts.c | 184 ++++++++++++++------------
 tests/generic/633                     |   1 -
 2 files changed, 98 insertions(+), 87 deletions(-)

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index a78a901f..5f304108 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -125,6 +125,9 @@ int t_dir1_fd;
 /* temporary buffer */
 char t_buf[PATH_MAX];
 
+/* whether the underlying filesystem supports idmapped mounts */
+bool t_fs_allow_idmap;
+
 static void stash_overflowuid(void)
 {
 	int fd;
@@ -2867,10 +2870,7 @@ out:
 static int fscaps(void)
 {
 	int fret = -1;
-	int file1_fd = -EBADF;
-	struct mount_attr attr = {
-		.attr_set = MOUNT_ATTR_IDMAP,
-	};
+	int file1_fd = -EBADF, fd_userns = -EBADF;
 	pid_t pid;
 
 	file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, 0644);
@@ -2884,8 +2884,8 @@ static int fscaps(void)
 		return 0;
 
 	/* Changing mount properties on a detached mount. */
-	attr.userns_fd	= get_userns_fd(0, 10000, 10000);
-	if (attr.userns_fd < 0) {
+	fd_userns = get_userns_fd(0, 10000, 10000);
+	if (fd_userns < 0) {
 		log_stderr("failure: get_userns_fd");
 		goto out;
 	}
@@ -2901,7 +2901,7 @@ static int fscaps(void)
 		goto out;
 	}
 	if (pid == 0) {
-		if (!switch_userns(attr.userns_fd, 0, 0, false))
+		if (!switch_userns(fd_userns, 0, 0, false))
 			die("failure: switch_userns");
 
 		/*
@@ -2949,7 +2949,7 @@ static int fscaps(void)
 		goto out;
 	}
 	if (pid == 0) {
-		if (!switch_userns(attr.userns_fd, 0, 0, false))
+		if (!switch_userns(fd_userns, 0, 0, false))
 			die("failure: switch_userns");
 
 		if (!expected_dummy_vfs_caps_uid(file1_fd, 0))
@@ -2977,8 +2977,8 @@ static int fscaps(void)
 	fret = 0;
 	log_debug("Ran test");
 out:
-	safe_close(attr.userns_fd);
 	safe_close(file1_fd);
+	safe_close(fd_userns);
 
 	return fret;
 }
@@ -13783,95 +13783,96 @@ static const struct option longopts[] = {
 
 struct t_idmapped_mounts {
 	int (*test)(void);
+	bool require_fs_allow_idmap;
 	const char *description;
 } basic_suite[] = {
-	{ acls,								"posix acls on regular mounts",									},
-	{ create_in_userns,						"create operations in user namespace",								},
-	{ device_node_in_userns,					"device node in user namespace",								},
-	{ expected_uid_gid_idmapped_mounts,				"expected ownership on idmapped mounts",							},
-	{ fscaps,							"fscaps on regular mounts",									},
-	{ fscaps_idmapped_mounts,					"fscaps on idmapped mounts",									},
-	{ fscaps_idmapped_mounts_in_userns,				"fscaps on idmapped mounts in user namespace",							},
-	{ fscaps_idmapped_mounts_in_userns_separate_userns,		"fscaps on idmapped mounts in user namespace with different id mappings",			},
-	{ fsids_mapped,							"mapped fsids",											},
-	{ fsids_unmapped,						"unmapped fsids",										},
-	{ hardlink_crossing_mounts,					"cross mount hardlink",										},
-	{ hardlink_crossing_idmapped_mounts,				"cross idmapped mount hardlink",								},
-	{ hardlink_from_idmapped_mount,					"hardlinks from idmapped mounts",								},
-	{ hardlink_from_idmapped_mount_in_userns,			"hardlinks from idmapped mounts in user namespace",						},
+	{ acls,								true,	"posix acls on regular mounts",									},
+	{ create_in_userns,						true,	"create operations in user namespace",								},
+	{ device_node_in_userns,					true,	"device node in user namespace",								},
+	{ expected_uid_gid_idmapped_mounts,				true,	"expected ownership on idmapped mounts",							},
+	{ fscaps,							false,	"fscaps on regular mounts",									},
+	{ fscaps_idmapped_mounts,					true,	"fscaps on idmapped mounts",									},
+	{ fscaps_idmapped_mounts_in_userns,				true,	"fscaps on idmapped mounts in user namespace",							},
+	{ fscaps_idmapped_mounts_in_userns_separate_userns,		true,	"fscaps on idmapped mounts in user namespace with different id mappings",			},
+	{ fsids_mapped,							true,	"mapped fsids",											},
+	{ fsids_unmapped,						true,	"unmapped fsids",										},
+	{ hardlink_crossing_mounts,					false,	"cross mount hardlink",										},
+	{ hardlink_crossing_idmapped_mounts,				true,	"cross idmapped mount hardlink",								},
+	{ hardlink_from_idmapped_mount,					true,	"hardlinks from idmapped mounts",								},
+	{ hardlink_from_idmapped_mount_in_userns,			true,	"hardlinks from idmapped mounts in user namespace",						},
 #ifdef HAVE_LIBURING_H
-	{ io_uring,							"io_uring",											},
-	{ io_uring_userns,						"io_uring in user namespace",									},
-	{ io_uring_idmapped,						"io_uring from idmapped mounts",								},
-	{ io_uring_idmapped_userns,					"io_uring from idmapped mounts in user namespace",						},
-	{ io_uring_idmapped_unmapped,					"io_uring from idmapped mounts with unmapped ids",						},
-	{ io_uring_idmapped_unmapped_userns,				"io_uring from idmapped mounts with unmapped ids in user namespace",				},
+	{ io_uring,							false,	"io_uring",											},
+	{ io_uring_userns,						false,	"io_uring in user namespace",									},
+	{ io_uring_idmapped,						true,	"io_uring from idmapped mounts",								},
+	{ io_uring_idmapped_userns,					true,	"io_uring from idmapped mounts in user namespace",						},
+	{ io_uring_idmapped_unmapped,					true,	"io_uring from idmapped mounts with unmapped ids",						},
+	{ io_uring_idmapped_unmapped_userns,				true,	"io_uring from idmapped mounts with unmapped ids in user namespace",				},
 #endif
-	{ protected_symlinks,						"following protected symlinks on regular mounts",						},
-	{ protected_symlinks_idmapped_mounts,				"following protected symlinks on idmapped mounts",						},
-	{ protected_symlinks_idmapped_mounts_in_userns,			"following protected symlinks on idmapped mounts in user namespace",				},
-	{ rename_crossing_mounts,					"cross mount rename",										},
-	{ rename_crossing_idmapped_mounts,				"cross idmapped mount rename",									},
-	{ rename_from_idmapped_mount,					"rename from idmapped mounts",									},
-	{ rename_from_idmapped_mount_in_userns,				"rename from idmapped mounts in user namespace",						},
-	{ setattr_truncate,						"setattr truncate",										},
-	{ setattr_truncate_idmapped,					"setattr truncate on idmapped mounts",								},
-	{ setattr_truncate_idmapped_in_userns,				"setattr truncate on idmapped mounts in user namespace",					},
-	{ setgid_create,						"create operations in directories with setgid bit set",						},
-	{ setgid_create_idmapped,					"create operations in directories with setgid bit set on idmapped mounts",			},
-	{ setgid_create_idmapped_in_userns,				"create operations in directories with setgid bit set on idmapped mounts in user namespace",	},
-	{ setid_binaries,						"setid binaries on regular mounts",								},
-	{ setid_binaries_idmapped_mounts,				"setid binaries on idmapped mounts",								},
-	{ setid_binaries_idmapped_mounts_in_userns,			"setid binaries on idmapped mounts in user namespace",						},
-	{ setid_binaries_idmapped_mounts_in_userns_separate_userns,	"setid binaries on idmapped mounts in user namespace with different id mappings",		},
-	{ sticky_bit_unlink,						"sticky bit unlink operations on regular mounts",						},
-	{ sticky_bit_unlink_idmapped_mounts,				"sticky bit unlink operations on idmapped mounts",						},
-	{ sticky_bit_unlink_idmapped_mounts_in_userns,			"sticky bit unlink operations on idmapped mounts in user namespace",				},
-	{ sticky_bit_rename,						"sticky bit rename operations on regular mounts",						},
-	{ sticky_bit_rename_idmapped_mounts,				"sticky bit rename operations on idmapped mounts",						},
-	{ sticky_bit_rename_idmapped_mounts_in_userns,			"sticky bit rename operations on idmapped mounts in user namespace",				},
-	{ symlink_regular_mounts,					"symlink from regular mounts",									},
-	{ symlink_idmapped_mounts,					"symlink from idmapped mounts",									},
-	{ symlink_idmapped_mounts_in_userns,				"symlink from idmapped mounts in user namespace",						},
-	{ threaded_idmapped_mount_interactions,				"threaded operations on idmapped mounts",							},
+	{ protected_symlinks,						false,	"following protected symlinks on regular mounts",						},
+	{ protected_symlinks_idmapped_mounts,				true,	"following protected symlinks on idmapped mounts",						},
+	{ protected_symlinks_idmapped_mounts_in_userns,			true,	"following protected symlinks on idmapped mounts in user namespace",				},
+	{ rename_crossing_mounts,					false,	"cross mount rename",										},
+	{ rename_crossing_idmapped_mounts,				true,	"cross idmapped mount rename",									},
+	{ rename_from_idmapped_mount,					true,	"rename from idmapped mounts",									},
+	{ rename_from_idmapped_mount_in_userns,				true,	"rename from idmapped mounts in user namespace",						},
+	{ setattr_truncate,						false,	"setattr truncate",										},
+	{ setattr_truncate_idmapped,					true,	"setattr truncate on idmapped mounts",								},
+	{ setattr_truncate_idmapped_in_userns,				true,	"setattr truncate on idmapped mounts in user namespace",					},
+	{ setgid_create,						false,	"create operations in directories with setgid bit set",						},
+	{ setgid_create_idmapped,					true,	"create operations in directories with setgid bit set on idmapped mounts",			},
+	{ setgid_create_idmapped_in_userns,				true,	"create operations in directories with setgid bit set on idmapped mounts in user namespace",	},
+	{ setid_binaries,						false,	"setid binaries on regular mounts",								},
+	{ setid_binaries_idmapped_mounts,				true,	"setid binaries on idmapped mounts",								},
+	{ setid_binaries_idmapped_mounts_in_userns,			true,	"setid binaries on idmapped mounts in user namespace",						},
+	{ setid_binaries_idmapped_mounts_in_userns_separate_userns,	true,	"setid binaries on idmapped mounts in user namespace with different id mappings",		},
+	{ sticky_bit_unlink,						false,	"sticky bit unlink operations on regular mounts",						},
+	{ sticky_bit_unlink_idmapped_mounts,				true,	"sticky bit unlink operations on idmapped mounts",						},
+	{ sticky_bit_unlink_idmapped_mounts_in_userns,			true,	"sticky bit unlink operations on idmapped mounts in user namespace",				},
+	{ sticky_bit_rename,						false,	"sticky bit rename operations on regular mounts",						},
+	{ sticky_bit_rename_idmapped_mounts,				true,	"sticky bit rename operations on idmapped mounts",						},
+	{ sticky_bit_rename_idmapped_mounts_in_userns,			true,	"sticky bit rename operations on idmapped mounts in user namespace",				},
+	{ symlink_regular_mounts,					false,	"symlink from regular mounts",									},
+	{ 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",							},
 };
 
 struct t_idmapped_mounts fscaps_in_ancestor_userns[] = {
-	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
+	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	true,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
 };
 
 struct t_idmapped_mounts t_nested_userns[] = {
-	{ nested_userns,						"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
+	{ nested_userns,						true,	"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
 };
 
 struct t_idmapped_mounts t_btrfs[] = {
-	{ btrfs_subvolumes_fsids_mapped,				"test subvolumes with mapped fsids",								},
-	{ btrfs_subvolumes_fsids_mapped_userns,				"test subvolumes with mapped fsids inside user namespace",					},
-	{ btrfs_subvolumes_fsids_mapped_user_subvol_rm_allowed,		"test subvolume deletion with user_subvol_rm_allowed mount option",				},
-	{ btrfs_subvolumes_fsids_mapped_userns_user_subvol_rm_allowed,	"test subvolume deletion with user_subvol_rm_allowed mount option inside user namespace",	},
-	{ btrfs_subvolumes_fsids_unmapped,				"test subvolumes with unmapped fsids",								},
-	{ btrfs_subvolumes_fsids_unmapped_userns,			"test subvolumes with unmapped fsids inside user namespace",					},
-	{ btrfs_snapshots_fsids_mapped,					"test snapshots with mapped fsids",								},
-	{ btrfs_snapshots_fsids_mapped_userns,				"test snapshots with mapped fsids inside user namespace",					},
-	{ btrfs_snapshots_fsids_mapped_user_subvol_rm_allowed,		"test snapshots deletion with user_subvol_rm_allowed mount option",				},
-	{ btrfs_snapshots_fsids_mapped_userns_user_subvol_rm_allowed,	"test snapshots deletion with user_subvol_rm_allowed mount option inside user namespace",	},
-	{ btrfs_snapshots_fsids_unmapped,				"test snapshots with unmapped fsids",								},
-	{ btrfs_snapshots_fsids_unmapped_userns,			"test snapshots with unmapped fsids inside user namespace",					},
-	{ btrfs_delete_by_spec_id,					"test subvolume deletion by spec id",								},
-	{ btrfs_subvolumes_setflags_fsids_mapped,			"test subvolume flags with mapped fsids",							},
-	{ btrfs_subvolumes_setflags_fsids_mapped_userns,		"test subvolume flags with mapped fsids inside user namespace",					},
-	{ btrfs_subvolumes_setflags_fsids_unmapped,			"test subvolume flags with unmapped fsids",							},
-	{ btrfs_subvolumes_setflags_fsids_unmapped_userns,		"test subvolume flags with unmapped fsids inside user namespace",				},
-	{ btrfs_snapshots_setflags_fsids_mapped,			"test snapshots flags with mapped fsids",							},
-	{ btrfs_snapshots_setflags_fsids_mapped_userns,			"test snapshots flags with mapped fsids inside user namespace",					},
-	{ btrfs_snapshots_setflags_fsids_unmapped,			"test snapshots flags with unmapped fsids",							},
-	{ btrfs_snapshots_setflags_fsids_unmapped_userns,		"test snapshots flags with unmapped fsids inside user namespace",				},
-	{ btrfs_subvolume_lookup_user,					"test unprivileged subvolume lookup",								},
+	{ btrfs_subvolumes_fsids_mapped,				true,	"test subvolumes with mapped fsids",								},
+	{ btrfs_subvolumes_fsids_mapped_userns,				true, 	"test subvolumes with mapped fsids inside user namespace",					},
+	{ btrfs_subvolumes_fsids_mapped_user_subvol_rm_allowed,		true, 	"test subvolume deletion with user_subvol_rm_allowed mount option",				},
+	{ btrfs_subvolumes_fsids_mapped_userns_user_subvol_rm_allowed,	true, 	"test subvolume deletion with user_subvol_rm_allowed mount option inside user namespace",	},
+	{ btrfs_subvolumes_fsids_unmapped,				true, 	"test subvolumes with unmapped fsids",								},
+	{ btrfs_subvolumes_fsids_unmapped_userns,			true, 	"test subvolumes with unmapped fsids inside user namespace",					},
+	{ btrfs_snapshots_fsids_mapped,					true, 	"test snapshots with mapped fsids",								},
+	{ btrfs_snapshots_fsids_mapped_userns,				true, 	"test snapshots with mapped fsids inside user namespace",					},
+	{ btrfs_snapshots_fsids_mapped_user_subvol_rm_allowed,		true, 	"test snapshots deletion with user_subvol_rm_allowed mount option",				},
+	{ btrfs_snapshots_fsids_mapped_userns_user_subvol_rm_allowed,	true, 	"test snapshots deletion with user_subvol_rm_allowed mount option inside user namespace",	},
+	{ btrfs_snapshots_fsids_unmapped,				true, 	"test snapshots with unmapped fsids",								},
+	{ btrfs_snapshots_fsids_unmapped_userns,			true, 	"test snapshots with unmapped fsids inside user namespace",					},
+	{ btrfs_delete_by_spec_id,					true, 	"test subvolume deletion by spec id",								},
+	{ btrfs_subvolumes_setflags_fsids_mapped,			true, 	"test subvolume flags with mapped fsids",							},
+	{ btrfs_subvolumes_setflags_fsids_mapped_userns,		true, 	"test subvolume flags with mapped fsids inside user namespace",					},
+	{ btrfs_subvolumes_setflags_fsids_unmapped,			true, 	"test subvolume flags with unmapped fsids",							},
+	{ btrfs_subvolumes_setflags_fsids_unmapped_userns,		true, 	"test subvolume flags with unmapped fsids inside user namespace",				},
+	{ btrfs_snapshots_setflags_fsids_mapped,			true, 	"test snapshots flags with mapped fsids",							},
+	{ btrfs_snapshots_setflags_fsids_mapped_userns,			true, 	"test snapshots flags with mapped fsids inside user namespace",					},
+	{ btrfs_snapshots_setflags_fsids_unmapped,			true, 	"test snapshots flags with unmapped fsids",							},
+	{ btrfs_snapshots_setflags_fsids_unmapped_userns,		true, 	"test snapshots flags with unmapped fsids inside user namespace",				},
+	{ btrfs_subvolume_lookup_user,					true, 	"test unprivileged subvolume lookup",								},
 };
 
 /* Test for commit 968219708108 ("fs: handle circular mappings correctly"). */
 struct t_idmapped_mounts t_setattr_fix_968219708108[] = {
-	{ setattr_fix_968219708108,					"test that setattr works correctly",								},
+	{ setattr_fix_968219708108,					true,	"test that setattr works correctly",								},
 };
 
 static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
@@ -13883,6 +13884,15 @@ static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
 		int ret;
 		pid_t pid;
 
+		/*
+		 * If the underlying filesystems does not support idmapped
+		 * mounts only run vfs generic tests.
+		 */
+		if (t->require_fs_allow_idmap && !t_fs_allow_idmap) {
+			log_debug("Skipping test %s", t->description);
+			continue;
+		}
+
 		test_setup();
 
 		pid = fork();
@@ -14011,13 +14021,15 @@ int main(int argc, char *argv[])
 	if (t_mnt_fd < 0)
 		die("failed to open %s", t_mountpoint_scratch);
 
-	/*
-	 * Caller just wants to know whether the filesystem we're on supports
-	 * idmapped mounts.
-	 */
+	t_fs_allow_idmap = fs_allow_idmap();
 	if (supported) {
-		if (!fs_allow_idmap())
+		/*
+		 * Caller just wants to know whether the filesystem we're on
+		 * supports idmapped mounts.
+		 */
+		if (!t_fs_allow_idmap)
 			exit(EXIT_FAILURE);
+
 		exit(EXIT_SUCCESS);
 	}
 
diff --git a/tests/generic/633 b/tests/generic/633
index 67501177..38280647 100755
--- a/tests/generic/633
+++ b/tests/generic/633
@@ -15,7 +15,6 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
 # real QA test starts here
 
 _supported_fs generic
-_require_idmapped_mounts
 _require_test
 
 echo "Silence is golden"
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper
  2022-01-13 13:24 [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper Christian Brauner
  2022-01-13 13:24 ` [PATCH 2/2] idmapped-mounts: always run generic vfs tests Christian Brauner
@ 2022-01-16  4:51 ` Eryu Guan
  2022-01-18 13:33   ` Christian Brauner
  1 sibling, 1 reply; 7+ messages in thread
From: Eryu Guan @ 2022-01-16  4:51 UTC (permalink / raw)
  To: Christian Brauner; +Cc: fstests, Christoph Hellwig, Seth Forshee, Eryu Guan

On Thu, Jan 13, 2022 at 02:24:20PM +0100, Christian Brauner wrote:
> Move the check whether the underlying filesystem supports idmapped
> mounts into a separate helper. We will use it in the following patch to
> make it possible to always run all tests that don't require idmapped
> mounts.
> 
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Eryu Guan <guaneryu@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: fstests@vger.kernel.org
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  src/idmapped-mounts/idmapped-mounts.c | 57 ++++++++++++++-------------
>  1 file changed, 30 insertions(+), 27 deletions(-)
> 
> diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
> index da690779..a78a901f 100644
> --- a/src/idmapped-mounts/idmapped-mounts.c
> +++ b/src/idmapped-mounts/idmapped-mounts.c
> @@ -13907,6 +13907,35 @@ static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
>  	return true;
>  }
>  
> +static bool fs_allow_idmap(void)
> +{
> +	int ret;
> +	int open_tree_fd = -EBADF;
> +	struct mount_attr attr = {
> +		.attr_set	= MOUNT_ATTR_IDMAP,
> +		.userns_fd	= -EBADF,
> +	};
> +
> +	/* Changing mount properties on a detached mount. */
> +	attr.userns_fd = get_userns_fd(0, 1000, 1);
> +	if (attr.userns_fd < 0)
> +		exit(EXIT_FAILURE);

IMHO, it'd be better to return false here and let caller decide if we
should exit or not.

Thanks,
Eryu

> +
> +	open_tree_fd = sys_open_tree(t_mnt_fd, "",
> +				     AT_EMPTY_PATH | AT_NO_AUTOMOUNT |
> +				     AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC |
> +				     OPEN_TREE_CLONE);
> +	if (open_tree_fd < 0)
> +		ret = -1;
> +	else
> +		ret = sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr,
> +					sizeof(attr));
> +	close(open_tree_fd);
> +	close(attr.userns_fd);
> +
> +	return ret == 0;
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	int fret, ret;
> @@ -13987,34 +14016,8 @@ int main(int argc, char *argv[])
>  	 * idmapped mounts.
>  	 */
>  	if (supported) {
> -		int open_tree_fd = -EBADF;
> -		struct mount_attr attr = {
> -			.attr_set	= MOUNT_ATTR_IDMAP,
> -			.userns_fd	= -EBADF,
> -		};
> -
> -		/* Changing mount properties on a detached mount. */
> -		attr.userns_fd	= get_userns_fd(0, 1000, 1);
> -		if (attr.userns_fd < 0)
> +		if (!fs_allow_idmap())
>  			exit(EXIT_FAILURE);
> -
> -		open_tree_fd = sys_open_tree(t_mnt_fd, "",
> -					     AT_EMPTY_PATH |
> -					     AT_NO_AUTOMOUNT |
> -					     AT_SYMLINK_NOFOLLOW |
> -					     OPEN_TREE_CLOEXEC |
> -					     OPEN_TREE_CLONE);
> -		if (open_tree_fd < 0)
> -			ret = -1;
> -		else
> -			ret = sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr, sizeof(attr));
> -
> -		close(open_tree_fd);
> -		close(attr.userns_fd);
> -
> -		if (ret)
> -			exit(EXIT_FAILURE);
> -
>  		exit(EXIT_SUCCESS);
>  	}
>  
> 
> base-commit: c61c1e2f4cad89d4f900eaef173616b309228110
> -- 
> 2.32.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] idmapped-mounts: always run generic vfs tests
  2022-01-13 13:24 ` [PATCH 2/2] idmapped-mounts: always run generic vfs tests Christian Brauner
@ 2022-01-16  4:52   ` Eryu Guan
  2022-01-18 13:37     ` Christian Brauner
  0 siblings, 1 reply; 7+ messages in thread
From: Eryu Guan @ 2022-01-16  4:52 UTC (permalink / raw)
  To: Christian Brauner; +Cc: fstests, Christoph Hellwig, Seth Forshee, Eryu Guan

On Thu, Jan 13, 2022 at 02:24:21PM +0100, Christian Brauner wrote:
> Make it possible to always run all the tests in the testsuite that don't
> require idmapped mounts. Now all filesystems can benefit from the
> generic vfs tests that we currently implement. This means setgid
> inheritance and other tests will be run for all filesystems not matter
> if they support idmapped mounts or not.
> 
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Eryu Guan <guaneryu@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: fstests@vger.kernel.org
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  src/idmapped-mounts/idmapped-mounts.c | 184 ++++++++++++++------------
>  tests/generic/633                     |   1 -
>  2 files changed, 98 insertions(+), 87 deletions(-)
> 
> diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
> index a78a901f..5f304108 100644
> --- a/src/idmapped-mounts/idmapped-mounts.c
> +++ b/src/idmapped-mounts/idmapped-mounts.c
> @@ -125,6 +125,9 @@ int t_dir1_fd;
>  /* temporary buffer */
>  char t_buf[PATH_MAX];
>  
> +/* whether the underlying filesystem supports idmapped mounts */
> +bool t_fs_allow_idmap;
> +
>  static void stash_overflowuid(void)
>  {
>  	int fd;
> @@ -2867,10 +2870,7 @@ out:
>  static int fscaps(void)
>  {
>  	int fret = -1;
> -	int file1_fd = -EBADF;
> -	struct mount_attr attr = {
> -		.attr_set = MOUNT_ATTR_IDMAP,
> -	};
> +	int file1_fd = -EBADF, fd_userns = -EBADF;
>  	pid_t pid;
>  
>  	file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, 0644);
> @@ -2884,8 +2884,8 @@ static int fscaps(void)
>  		return 0;
>  
>  	/* Changing mount properties on a detached mount. */
> -	attr.userns_fd	= get_userns_fd(0, 10000, 10000);
> -	if (attr.userns_fd < 0) {
> +	fd_userns = get_userns_fd(0, 10000, 10000);
> +	if (fd_userns < 0) {
>  		log_stderr("failure: get_userns_fd");
>  		goto out;
>  	}
> @@ -2901,7 +2901,7 @@ static int fscaps(void)
>  		goto out;
>  	}
>  	if (pid == 0) {
> -		if (!switch_userns(attr.userns_fd, 0, 0, false))
> +		if (!switch_userns(fd_userns, 0, 0, false))
>  			die("failure: switch_userns");
>  
>  		/*
> @@ -2949,7 +2949,7 @@ static int fscaps(void)
>  		goto out;
>  	}
>  	if (pid == 0) {
> -		if (!switch_userns(attr.userns_fd, 0, 0, false))
> +		if (!switch_userns(fd_userns, 0, 0, false))
>  			die("failure: switch_userns");
>  
>  		if (!expected_dummy_vfs_caps_uid(file1_fd, 0))
> @@ -2977,8 +2977,8 @@ static int fscaps(void)
>  	fret = 0;
>  	log_debug("Ran test");
>  out:
> -	safe_close(attr.userns_fd);
>  	safe_close(file1_fd);
> +	safe_close(fd_userns);
>  
>  	return fret;
>  }
> @@ -13783,95 +13783,96 @@ static const struct option longopts[] = {
>  
>  struct t_idmapped_mounts {
>  	int (*test)(void);
> +	bool require_fs_allow_idmap;
>  	const char *description;
>  } basic_suite[] = {
> -	{ acls,								"posix acls on regular mounts",									},
> -	{ create_in_userns,						"create operations in user namespace",								},
> -	{ device_node_in_userns,					"device node in user namespace",								},
> -	{ expected_uid_gid_idmapped_mounts,				"expected ownership on idmapped mounts",							},
> -	{ fscaps,							"fscaps on regular mounts",									},
> -	{ fscaps_idmapped_mounts,					"fscaps on idmapped mounts",									},
> -	{ fscaps_idmapped_mounts_in_userns,				"fscaps on idmapped mounts in user namespace",							},
> -	{ fscaps_idmapped_mounts_in_userns_separate_userns,		"fscaps on idmapped mounts in user namespace with different id mappings",			},
> -	{ fsids_mapped,							"mapped fsids",											},
> -	{ fsids_unmapped,						"unmapped fsids",										},
> -	{ hardlink_crossing_mounts,					"cross mount hardlink",										},
> -	{ hardlink_crossing_idmapped_mounts,				"cross idmapped mount hardlink",								},
> -	{ hardlink_from_idmapped_mount,					"hardlinks from idmapped mounts",								},
> -	{ hardlink_from_idmapped_mount_in_userns,			"hardlinks from idmapped mounts in user namespace",						},
> +	{ acls,								true,	"posix acls on regular mounts",									},
> +	{ create_in_userns,						true,	"create operations in user namespace",								},
> +	{ device_node_in_userns,					true,	"device node in user namespace",								},
> +	{ expected_uid_gid_idmapped_mounts,				true,	"expected ownership on idmapped mounts",							},
> +	{ fscaps,							false,	"fscaps on regular mounts",									},
> +	{ fscaps_idmapped_mounts,					true,	"fscaps on idmapped mounts",									},
> +	{ fscaps_idmapped_mounts_in_userns,				true,	"fscaps on idmapped mounts in user namespace",							},
> +	{ fscaps_idmapped_mounts_in_userns_separate_userns,		true,	"fscaps on idmapped mounts in user namespace with different id mappings",			},
> +	{ fsids_mapped,							true,	"mapped fsids",											},
> +	{ fsids_unmapped,						true,	"unmapped fsids",										},
> +	{ hardlink_crossing_mounts,					false,	"cross mount hardlink",										},
> +	{ hardlink_crossing_idmapped_mounts,				true,	"cross idmapped mount hardlink",								},
> +	{ hardlink_from_idmapped_mount,					true,	"hardlinks from idmapped mounts",								},
> +	{ hardlink_from_idmapped_mount_in_userns,			true,	"hardlinks from idmapped mounts in user namespace",						},
>  #ifdef HAVE_LIBURING_H
> -	{ io_uring,							"io_uring",											},
> -	{ io_uring_userns,						"io_uring in user namespace",									},
> -	{ io_uring_idmapped,						"io_uring from idmapped mounts",								},
> -	{ io_uring_idmapped_userns,					"io_uring from idmapped mounts in user namespace",						},
> -	{ io_uring_idmapped_unmapped,					"io_uring from idmapped mounts with unmapped ids",						},
> -	{ io_uring_idmapped_unmapped_userns,				"io_uring from idmapped mounts with unmapped ids in user namespace",				},
> +	{ io_uring,							false,	"io_uring",											},
> +	{ io_uring_userns,						false,	"io_uring in user namespace",									},
> +	{ io_uring_idmapped,						true,	"io_uring from idmapped mounts",								},
> +	{ io_uring_idmapped_userns,					true,	"io_uring from idmapped mounts in user namespace",						},
> +	{ io_uring_idmapped_unmapped,					true,	"io_uring from idmapped mounts with unmapped ids",						},
> +	{ io_uring_idmapped_unmapped_userns,				true,	"io_uring from idmapped mounts with unmapped ids in user namespace",				},
>  #endif
> -	{ protected_symlinks,						"following protected symlinks on regular mounts",						},
> -	{ protected_symlinks_idmapped_mounts,				"following protected symlinks on idmapped mounts",						},
> -	{ protected_symlinks_idmapped_mounts_in_userns,			"following protected symlinks on idmapped mounts in user namespace",				},
> -	{ rename_crossing_mounts,					"cross mount rename",										},
> -	{ rename_crossing_idmapped_mounts,				"cross idmapped mount rename",									},
> -	{ rename_from_idmapped_mount,					"rename from idmapped mounts",									},
> -	{ rename_from_idmapped_mount_in_userns,				"rename from idmapped mounts in user namespace",						},
> -	{ setattr_truncate,						"setattr truncate",										},
> -	{ setattr_truncate_idmapped,					"setattr truncate on idmapped mounts",								},
> -	{ setattr_truncate_idmapped_in_userns,				"setattr truncate on idmapped mounts in user namespace",					},
> -	{ setgid_create,						"create operations in directories with setgid bit set",						},
> -	{ setgid_create_idmapped,					"create operations in directories with setgid bit set on idmapped mounts",			},
> -	{ setgid_create_idmapped_in_userns,				"create operations in directories with setgid bit set on idmapped mounts in user namespace",	},
> -	{ setid_binaries,						"setid binaries on regular mounts",								},
> -	{ setid_binaries_idmapped_mounts,				"setid binaries on idmapped mounts",								},
> -	{ setid_binaries_idmapped_mounts_in_userns,			"setid binaries on idmapped mounts in user namespace",						},
> -	{ setid_binaries_idmapped_mounts_in_userns_separate_userns,	"setid binaries on idmapped mounts in user namespace with different id mappings",		},
> -	{ sticky_bit_unlink,						"sticky bit unlink operations on regular mounts",						},
> -	{ sticky_bit_unlink_idmapped_mounts,				"sticky bit unlink operations on idmapped mounts",						},
> -	{ sticky_bit_unlink_idmapped_mounts_in_userns,			"sticky bit unlink operations on idmapped mounts in user namespace",				},
> -	{ sticky_bit_rename,						"sticky bit rename operations on regular mounts",						},
> -	{ sticky_bit_rename_idmapped_mounts,				"sticky bit rename operations on idmapped mounts",						},
> -	{ sticky_bit_rename_idmapped_mounts_in_userns,			"sticky bit rename operations on idmapped mounts in user namespace",				},
> -	{ symlink_regular_mounts,					"symlink from regular mounts",									},
> -	{ symlink_idmapped_mounts,					"symlink from idmapped mounts",									},
> -	{ symlink_idmapped_mounts_in_userns,				"symlink from idmapped mounts in user namespace",						},
> -	{ threaded_idmapped_mount_interactions,				"threaded operations on idmapped mounts",							},
> +	{ protected_symlinks,						false,	"following protected symlinks on regular mounts",						},
> +	{ protected_symlinks_idmapped_mounts,				true,	"following protected symlinks on idmapped mounts",						},
> +	{ protected_symlinks_idmapped_mounts_in_userns,			true,	"following protected symlinks on idmapped mounts in user namespace",				},
> +	{ rename_crossing_mounts,					false,	"cross mount rename",										},
> +	{ rename_crossing_idmapped_mounts,				true,	"cross idmapped mount rename",									},
> +	{ rename_from_idmapped_mount,					true,	"rename from idmapped mounts",									},
> +	{ rename_from_idmapped_mount_in_userns,				true,	"rename from idmapped mounts in user namespace",						},
> +	{ setattr_truncate,						false,	"setattr truncate",										},
> +	{ setattr_truncate_idmapped,					true,	"setattr truncate on idmapped mounts",								},
> +	{ setattr_truncate_idmapped_in_userns,				true,	"setattr truncate on idmapped mounts in user namespace",					},
> +	{ setgid_create,						false,	"create operations in directories with setgid bit set",						},
> +	{ setgid_create_idmapped,					true,	"create operations in directories with setgid bit set on idmapped mounts",			},
> +	{ setgid_create_idmapped_in_userns,				true,	"create operations in directories with setgid bit set on idmapped mounts in user namespace",	},
> +	{ setid_binaries,						false,	"setid binaries on regular mounts",								},
> +	{ setid_binaries_idmapped_mounts,				true,	"setid binaries on idmapped mounts",								},
> +	{ setid_binaries_idmapped_mounts_in_userns,			true,	"setid binaries on idmapped mounts in user namespace",						},
> +	{ setid_binaries_idmapped_mounts_in_userns_separate_userns,	true,	"setid binaries on idmapped mounts in user namespace with different id mappings",		},
> +	{ sticky_bit_unlink,						false,	"sticky bit unlink operations on regular mounts",						},
> +	{ sticky_bit_unlink_idmapped_mounts,				true,	"sticky bit unlink operations on idmapped mounts",						},
> +	{ sticky_bit_unlink_idmapped_mounts_in_userns,			true,	"sticky bit unlink operations on idmapped mounts in user namespace",				},
> +	{ sticky_bit_rename,						false,	"sticky bit rename operations on regular mounts",						},
> +	{ sticky_bit_rename_idmapped_mounts,				true,	"sticky bit rename operations on idmapped mounts",						},
> +	{ sticky_bit_rename_idmapped_mounts_in_userns,			true,	"sticky bit rename operations on idmapped mounts in user namespace",				},
> +	{ symlink_regular_mounts,					false,	"symlink from regular mounts",									},
> +	{ 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",							},
>  };
>  
>  struct t_idmapped_mounts fscaps_in_ancestor_userns[] = {
> -	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
> +	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	true,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
>  };
>  
>  struct t_idmapped_mounts t_nested_userns[] = {
> -	{ nested_userns,						"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
> +	{ nested_userns,						true,	"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
>  };
>  
>  struct t_idmapped_mounts t_btrfs[] = {
> -	{ btrfs_subvolumes_fsids_mapped,				"test subvolumes with mapped fsids",								},
> -	{ btrfs_subvolumes_fsids_mapped_userns,				"test subvolumes with mapped fsids inside user namespace",					},
> -	{ btrfs_subvolumes_fsids_mapped_user_subvol_rm_allowed,		"test subvolume deletion with user_subvol_rm_allowed mount option",				},
> -	{ btrfs_subvolumes_fsids_mapped_userns_user_subvol_rm_allowed,	"test subvolume deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> -	{ btrfs_subvolumes_fsids_unmapped,				"test subvolumes with unmapped fsids",								},
> -	{ btrfs_subvolumes_fsids_unmapped_userns,			"test subvolumes with unmapped fsids inside user namespace",					},
> -	{ btrfs_snapshots_fsids_mapped,					"test snapshots with mapped fsids",								},
> -	{ btrfs_snapshots_fsids_mapped_userns,				"test snapshots with mapped fsids inside user namespace",					},
> -	{ btrfs_snapshots_fsids_mapped_user_subvol_rm_allowed,		"test snapshots deletion with user_subvol_rm_allowed mount option",				},
> -	{ btrfs_snapshots_fsids_mapped_userns_user_subvol_rm_allowed,	"test snapshots deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> -	{ btrfs_snapshots_fsids_unmapped,				"test snapshots with unmapped fsids",								},
> -	{ btrfs_snapshots_fsids_unmapped_userns,			"test snapshots with unmapped fsids inside user namespace",					},
> -	{ btrfs_delete_by_spec_id,					"test subvolume deletion by spec id",								},
> -	{ btrfs_subvolumes_setflags_fsids_mapped,			"test subvolume flags with mapped fsids",							},
> -	{ btrfs_subvolumes_setflags_fsids_mapped_userns,		"test subvolume flags with mapped fsids inside user namespace",					},
> -	{ btrfs_subvolumes_setflags_fsids_unmapped,			"test subvolume flags with unmapped fsids",							},
> -	{ btrfs_subvolumes_setflags_fsids_unmapped_userns,		"test subvolume flags with unmapped fsids inside user namespace",				},
> -	{ btrfs_snapshots_setflags_fsids_mapped,			"test snapshots flags with mapped fsids",							},
> -	{ btrfs_snapshots_setflags_fsids_mapped_userns,			"test snapshots flags with mapped fsids inside user namespace",					},
> -	{ btrfs_snapshots_setflags_fsids_unmapped,			"test snapshots flags with unmapped fsids",							},
> -	{ btrfs_snapshots_setflags_fsids_unmapped_userns,		"test snapshots flags with unmapped fsids inside user namespace",				},
> -	{ btrfs_subvolume_lookup_user,					"test unprivileged subvolume lookup",								},
> +	{ btrfs_subvolumes_fsids_mapped,				true,	"test subvolumes with mapped fsids",								},
> +	{ btrfs_subvolumes_fsids_mapped_userns,				true, 	"test subvolumes with mapped fsids inside user namespace",					},
> +	{ btrfs_subvolumes_fsids_mapped_user_subvol_rm_allowed,		true, 	"test subvolume deletion with user_subvol_rm_allowed mount option",				},
> +	{ btrfs_subvolumes_fsids_mapped_userns_user_subvol_rm_allowed,	true, 	"test subvolume deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> +	{ btrfs_subvolumes_fsids_unmapped,				true, 	"test subvolumes with unmapped fsids",								},
> +	{ btrfs_subvolumes_fsids_unmapped_userns,			true, 	"test subvolumes with unmapped fsids inside user namespace",					},
> +	{ btrfs_snapshots_fsids_mapped,					true, 	"test snapshots with mapped fsids",								},
> +	{ btrfs_snapshots_fsids_mapped_userns,				true, 	"test snapshots with mapped fsids inside user namespace",					},
> +	{ btrfs_snapshots_fsids_mapped_user_subvol_rm_allowed,		true, 	"test snapshots deletion with user_subvol_rm_allowed mount option",				},
> +	{ btrfs_snapshots_fsids_mapped_userns_user_subvol_rm_allowed,	true, 	"test snapshots deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> +	{ btrfs_snapshots_fsids_unmapped,				true, 	"test snapshots with unmapped fsids",								},
> +	{ btrfs_snapshots_fsids_unmapped_userns,			true, 	"test snapshots with unmapped fsids inside user namespace",					},
> +	{ btrfs_delete_by_spec_id,					true, 	"test subvolume deletion by spec id",								},
> +	{ btrfs_subvolumes_setflags_fsids_mapped,			true, 	"test subvolume flags with mapped fsids",							},
> +	{ btrfs_subvolumes_setflags_fsids_mapped_userns,		true, 	"test subvolume flags with mapped fsids inside user namespace",					},
> +	{ btrfs_subvolumes_setflags_fsids_unmapped,			true, 	"test subvolume flags with unmapped fsids",							},
> +	{ btrfs_subvolumes_setflags_fsids_unmapped_userns,		true, 	"test subvolume flags with unmapped fsids inside user namespace",				},
> +	{ btrfs_snapshots_setflags_fsids_mapped,			true, 	"test snapshots flags with mapped fsids",							},
> +	{ btrfs_snapshots_setflags_fsids_mapped_userns,			true, 	"test snapshots flags with mapped fsids inside user namespace",					},
> +	{ btrfs_snapshots_setflags_fsids_unmapped,			true, 	"test snapshots flags with unmapped fsids",							},
> +	{ btrfs_snapshots_setflags_fsids_unmapped_userns,		true, 	"test snapshots flags with unmapped fsids inside user namespace",				},
> +	{ btrfs_subvolume_lookup_user,					true, 	"test unprivileged subvolume lookup",								},
>  };
>  
>  /* Test for commit 968219708108 ("fs: handle circular mappings correctly"). */
>  struct t_idmapped_mounts t_setattr_fix_968219708108[] = {
> -	{ setattr_fix_968219708108,					"test that setattr works correctly",								},
> +	{ setattr_fix_968219708108,					true,	"test that setattr works correctly",								},
>  };
>  
>  static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
> @@ -13883,6 +13884,15 @@ static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
>  		int ret;
>  		pid_t pid;
>  
> +		/*
> +		 * If the underlying filesystems does not support idmapped
> +		 * mounts only run vfs generic tests.
> +		 */
> +		if (t->require_fs_allow_idmap && !t_fs_allow_idmap) {
> +			log_debug("Skipping test %s", t->description);
> +			continue;
> +		}
> +
>  		test_setup();
>  
>  		pid = fork();
> @@ -14011,13 +14021,15 @@ int main(int argc, char *argv[])
>  	if (t_mnt_fd < 0)
>  		die("failed to open %s", t_mountpoint_scratch);
>  
> -	/*
> -	 * Caller just wants to know whether the filesystem we're on supports
> -	 * idmapped mounts.
> -	 */
> +	t_fs_allow_idmap = fs_allow_idmap();
>  	if (supported) {
> -		if (!fs_allow_idmap())
> +		/*
> +		 * Caller just wants to know whether the filesystem we're on
> +		 * supports idmapped mounts.
> +		 */
> +		if (!t_fs_allow_idmap)
>  			exit(EXIT_FAILURE);
> +
>  		exit(EXIT_SUCCESS);
>  	}
>  
> diff --git a/tests/generic/633 b/tests/generic/633
> index 67501177..38280647 100755
> --- a/tests/generic/633
> +++ b/tests/generic/633
> @@ -15,7 +15,6 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
>  # real QA test starts here
>  
>  _supported_fs generic
> -_require_idmapped_mounts

Removed by accident or is it intentional?

Thanks,
Eryu

>  _require_test
>  
>  echo "Silence is golden"
> -- 
> 2.32.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper
  2022-01-16  4:51 ` [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper Eryu Guan
@ 2022-01-18 13:33   ` Christian Brauner
  0 siblings, 0 replies; 7+ messages in thread
From: Christian Brauner @ 2022-01-18 13:33 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, Christoph Hellwig, Seth Forshee, Eryu Guan

On Sun, Jan 16, 2022 at 12:51:32PM +0800, Eryu Guan wrote:
> On Thu, Jan 13, 2022 at 02:24:20PM +0100, Christian Brauner wrote:
> > Move the check whether the underlying filesystem supports idmapped
> > mounts into a separate helper. We will use it in the following patch to
> > make it possible to always run all tests that don't require idmapped
> > mounts.
> > 
> > Cc: Seth Forshee <sforshee@digitalocean.com>
> > Cc: Eryu Guan <guaneryu@gmail.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: fstests@vger.kernel.org
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  src/idmapped-mounts/idmapped-mounts.c | 57 ++++++++++++++-------------
> >  1 file changed, 30 insertions(+), 27 deletions(-)
> > 
> > diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
> > index da690779..a78a901f 100644
> > --- a/src/idmapped-mounts/idmapped-mounts.c
> > +++ b/src/idmapped-mounts/idmapped-mounts.c
> > @@ -13907,6 +13907,35 @@ static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
> >  	return true;
> >  }
> >  
> > +static bool fs_allow_idmap(void)
> > +{
> > +	int ret;
> > +	int open_tree_fd = -EBADF;
> > +	struct mount_attr attr = {
> > +		.attr_set	= MOUNT_ATTR_IDMAP,
> > +		.userns_fd	= -EBADF,
> > +	};
> > +
> > +	/* Changing mount properties on a detached mount. */
> > +	attr.userns_fd = get_userns_fd(0, 1000, 1);
> > +	if (attr.userns_fd < 0)
> > +		exit(EXIT_FAILURE);
> 
> IMHO, it'd be better to return false here and let caller decide if we
> should exit or not.

Agreed.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] idmapped-mounts: always run generic vfs tests
  2022-01-16  4:52   ` Eryu Guan
@ 2022-01-18 13:37     ` Christian Brauner
  2022-01-18 13:40       ` Eryu Guan
  0 siblings, 1 reply; 7+ messages in thread
From: Christian Brauner @ 2022-01-18 13:37 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, Christoph Hellwig, Seth Forshee, Eryu Guan

On Sun, Jan 16, 2022 at 12:52:33PM +0800, Eryu Guan wrote:
> On Thu, Jan 13, 2022 at 02:24:21PM +0100, Christian Brauner wrote:
> > Make it possible to always run all the tests in the testsuite that don't
> > require idmapped mounts. Now all filesystems can benefit from the
> > generic vfs tests that we currently implement. This means setgid
> > inheritance and other tests will be run for all filesystems not matter
> > if they support idmapped mounts or not.
> > 
> > Cc: Seth Forshee <sforshee@digitalocean.com>
> > Cc: Eryu Guan <guaneryu@gmail.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: fstests@vger.kernel.org
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  src/idmapped-mounts/idmapped-mounts.c | 184 ++++++++++++++------------
> >  tests/generic/633                     |   1 -
> >  2 files changed, 98 insertions(+), 87 deletions(-)
> > 
> > diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
> > index a78a901f..5f304108 100644
> > --- a/src/idmapped-mounts/idmapped-mounts.c
> > +++ b/src/idmapped-mounts/idmapped-mounts.c
> > @@ -125,6 +125,9 @@ int t_dir1_fd;
> >  /* temporary buffer */
> >  char t_buf[PATH_MAX];
> >  
> > +/* whether the underlying filesystem supports idmapped mounts */
> > +bool t_fs_allow_idmap;
> > +
> >  static void stash_overflowuid(void)
> >  {
> >  	int fd;
> > @@ -2867,10 +2870,7 @@ out:
> >  static int fscaps(void)
> >  {
> >  	int fret = -1;
> > -	int file1_fd = -EBADF;
> > -	struct mount_attr attr = {
> > -		.attr_set = MOUNT_ATTR_IDMAP,
> > -	};
> > +	int file1_fd = -EBADF, fd_userns = -EBADF;
> >  	pid_t pid;
> >  
> >  	file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, 0644);
> > @@ -2884,8 +2884,8 @@ static int fscaps(void)
> >  		return 0;
> >  
> >  	/* Changing mount properties on a detached mount. */
> > -	attr.userns_fd	= get_userns_fd(0, 10000, 10000);
> > -	if (attr.userns_fd < 0) {
> > +	fd_userns = get_userns_fd(0, 10000, 10000);
> > +	if (fd_userns < 0) {
> >  		log_stderr("failure: get_userns_fd");
> >  		goto out;
> >  	}
> > @@ -2901,7 +2901,7 @@ static int fscaps(void)
> >  		goto out;
> >  	}
> >  	if (pid == 0) {
> > -		if (!switch_userns(attr.userns_fd, 0, 0, false))
> > +		if (!switch_userns(fd_userns, 0, 0, false))
> >  			die("failure: switch_userns");
> >  
> >  		/*
> > @@ -2949,7 +2949,7 @@ static int fscaps(void)
> >  		goto out;
> >  	}
> >  	if (pid == 0) {
> > -		if (!switch_userns(attr.userns_fd, 0, 0, false))
> > +		if (!switch_userns(fd_userns, 0, 0, false))
> >  			die("failure: switch_userns");
> >  
> >  		if (!expected_dummy_vfs_caps_uid(file1_fd, 0))
> > @@ -2977,8 +2977,8 @@ static int fscaps(void)
> >  	fret = 0;
> >  	log_debug("Ran test");
> >  out:
> > -	safe_close(attr.userns_fd);
> >  	safe_close(file1_fd);
> > +	safe_close(fd_userns);
> >  
> >  	return fret;
> >  }
> > @@ -13783,95 +13783,96 @@ static const struct option longopts[] = {
> >  
> >  struct t_idmapped_mounts {
> >  	int (*test)(void);
> > +	bool require_fs_allow_idmap;
> >  	const char *description;
> >  } basic_suite[] = {
> > -	{ acls,								"posix acls on regular mounts",									},
> > -	{ create_in_userns,						"create operations in user namespace",								},
> > -	{ device_node_in_userns,					"device node in user namespace",								},
> > -	{ expected_uid_gid_idmapped_mounts,				"expected ownership on idmapped mounts",							},
> > -	{ fscaps,							"fscaps on regular mounts",									},
> > -	{ fscaps_idmapped_mounts,					"fscaps on idmapped mounts",									},
> > -	{ fscaps_idmapped_mounts_in_userns,				"fscaps on idmapped mounts in user namespace",							},
> > -	{ fscaps_idmapped_mounts_in_userns_separate_userns,		"fscaps on idmapped mounts in user namespace with different id mappings",			},
> > -	{ fsids_mapped,							"mapped fsids",											},
> > -	{ fsids_unmapped,						"unmapped fsids",										},
> > -	{ hardlink_crossing_mounts,					"cross mount hardlink",										},
> > -	{ hardlink_crossing_idmapped_mounts,				"cross idmapped mount hardlink",								},
> > -	{ hardlink_from_idmapped_mount,					"hardlinks from idmapped mounts",								},
> > -	{ hardlink_from_idmapped_mount_in_userns,			"hardlinks from idmapped mounts in user namespace",						},
> > +	{ acls,								true,	"posix acls on regular mounts",									},
> > +	{ create_in_userns,						true,	"create operations in user namespace",								},
> > +	{ device_node_in_userns,					true,	"device node in user namespace",								},
> > +	{ expected_uid_gid_idmapped_mounts,				true,	"expected ownership on idmapped mounts",							},
> > +	{ fscaps,							false,	"fscaps on regular mounts",									},
> > +	{ fscaps_idmapped_mounts,					true,	"fscaps on idmapped mounts",									},
> > +	{ fscaps_idmapped_mounts_in_userns,				true,	"fscaps on idmapped mounts in user namespace",							},
> > +	{ fscaps_idmapped_mounts_in_userns_separate_userns,		true,	"fscaps on idmapped mounts in user namespace with different id mappings",			},
> > +	{ fsids_mapped,							true,	"mapped fsids",											},
> > +	{ fsids_unmapped,						true,	"unmapped fsids",										},
> > +	{ hardlink_crossing_mounts,					false,	"cross mount hardlink",										},
> > +	{ hardlink_crossing_idmapped_mounts,				true,	"cross idmapped mount hardlink",								},
> > +	{ hardlink_from_idmapped_mount,					true,	"hardlinks from idmapped mounts",								},
> > +	{ hardlink_from_idmapped_mount_in_userns,			true,	"hardlinks from idmapped mounts in user namespace",						},
> >  #ifdef HAVE_LIBURING_H
> > -	{ io_uring,							"io_uring",											},
> > -	{ io_uring_userns,						"io_uring in user namespace",									},
> > -	{ io_uring_idmapped,						"io_uring from idmapped mounts",								},
> > -	{ io_uring_idmapped_userns,					"io_uring from idmapped mounts in user namespace",						},
> > -	{ io_uring_idmapped_unmapped,					"io_uring from idmapped mounts with unmapped ids",						},
> > -	{ io_uring_idmapped_unmapped_userns,				"io_uring from idmapped mounts with unmapped ids in user namespace",				},
> > +	{ io_uring,							false,	"io_uring",											},
> > +	{ io_uring_userns,						false,	"io_uring in user namespace",									},
> > +	{ io_uring_idmapped,						true,	"io_uring from idmapped mounts",								},
> > +	{ io_uring_idmapped_userns,					true,	"io_uring from idmapped mounts in user namespace",						},
> > +	{ io_uring_idmapped_unmapped,					true,	"io_uring from idmapped mounts with unmapped ids",						},
> > +	{ io_uring_idmapped_unmapped_userns,				true,	"io_uring from idmapped mounts with unmapped ids in user namespace",				},
> >  #endif
> > -	{ protected_symlinks,						"following protected symlinks on regular mounts",						},
> > -	{ protected_symlinks_idmapped_mounts,				"following protected symlinks on idmapped mounts",						},
> > -	{ protected_symlinks_idmapped_mounts_in_userns,			"following protected symlinks on idmapped mounts in user namespace",				},
> > -	{ rename_crossing_mounts,					"cross mount rename",										},
> > -	{ rename_crossing_idmapped_mounts,				"cross idmapped mount rename",									},
> > -	{ rename_from_idmapped_mount,					"rename from idmapped mounts",									},
> > -	{ rename_from_idmapped_mount_in_userns,				"rename from idmapped mounts in user namespace",						},
> > -	{ setattr_truncate,						"setattr truncate",										},
> > -	{ setattr_truncate_idmapped,					"setattr truncate on idmapped mounts",								},
> > -	{ setattr_truncate_idmapped_in_userns,				"setattr truncate on idmapped mounts in user namespace",					},
> > -	{ setgid_create,						"create operations in directories with setgid bit set",						},
> > -	{ setgid_create_idmapped,					"create operations in directories with setgid bit set on idmapped mounts",			},
> > -	{ setgid_create_idmapped_in_userns,				"create operations in directories with setgid bit set on idmapped mounts in user namespace",	},
> > -	{ setid_binaries,						"setid binaries on regular mounts",								},
> > -	{ setid_binaries_idmapped_mounts,				"setid binaries on idmapped mounts",								},
> > -	{ setid_binaries_idmapped_mounts_in_userns,			"setid binaries on idmapped mounts in user namespace",						},
> > -	{ setid_binaries_idmapped_mounts_in_userns_separate_userns,	"setid binaries on idmapped mounts in user namespace with different id mappings",		},
> > -	{ sticky_bit_unlink,						"sticky bit unlink operations on regular mounts",						},
> > -	{ sticky_bit_unlink_idmapped_mounts,				"sticky bit unlink operations on idmapped mounts",						},
> > -	{ sticky_bit_unlink_idmapped_mounts_in_userns,			"sticky bit unlink operations on idmapped mounts in user namespace",				},
> > -	{ sticky_bit_rename,						"sticky bit rename operations on regular mounts",						},
> > -	{ sticky_bit_rename_idmapped_mounts,				"sticky bit rename operations on idmapped mounts",						},
> > -	{ sticky_bit_rename_idmapped_mounts_in_userns,			"sticky bit rename operations on idmapped mounts in user namespace",				},
> > -	{ symlink_regular_mounts,					"symlink from regular mounts",									},
> > -	{ symlink_idmapped_mounts,					"symlink from idmapped mounts",									},
> > -	{ symlink_idmapped_mounts_in_userns,				"symlink from idmapped mounts in user namespace",						},
> > -	{ threaded_idmapped_mount_interactions,				"threaded operations on idmapped mounts",							},
> > +	{ protected_symlinks,						false,	"following protected symlinks on regular mounts",						},
> > +	{ protected_symlinks_idmapped_mounts,				true,	"following protected symlinks on idmapped mounts",						},
> > +	{ protected_symlinks_idmapped_mounts_in_userns,			true,	"following protected symlinks on idmapped mounts in user namespace",				},
> > +	{ rename_crossing_mounts,					false,	"cross mount rename",										},
> > +	{ rename_crossing_idmapped_mounts,				true,	"cross idmapped mount rename",									},
> > +	{ rename_from_idmapped_mount,					true,	"rename from idmapped mounts",									},
> > +	{ rename_from_idmapped_mount_in_userns,				true,	"rename from idmapped mounts in user namespace",						},
> > +	{ setattr_truncate,						false,	"setattr truncate",										},
> > +	{ setattr_truncate_idmapped,					true,	"setattr truncate on idmapped mounts",								},
> > +	{ setattr_truncate_idmapped_in_userns,				true,	"setattr truncate on idmapped mounts in user namespace",					},
> > +	{ setgid_create,						false,	"create operations in directories with setgid bit set",						},
> > +	{ setgid_create_idmapped,					true,	"create operations in directories with setgid bit set on idmapped mounts",			},
> > +	{ setgid_create_idmapped_in_userns,				true,	"create operations in directories with setgid bit set on idmapped mounts in user namespace",	},
> > +	{ setid_binaries,						false,	"setid binaries on regular mounts",								},
> > +	{ setid_binaries_idmapped_mounts,				true,	"setid binaries on idmapped mounts",								},
> > +	{ setid_binaries_idmapped_mounts_in_userns,			true,	"setid binaries on idmapped mounts in user namespace",						},
> > +	{ setid_binaries_idmapped_mounts_in_userns_separate_userns,	true,	"setid binaries on idmapped mounts in user namespace with different id mappings",		},
> > +	{ sticky_bit_unlink,						false,	"sticky bit unlink operations on regular mounts",						},
> > +	{ sticky_bit_unlink_idmapped_mounts,				true,	"sticky bit unlink operations on idmapped mounts",						},
> > +	{ sticky_bit_unlink_idmapped_mounts_in_userns,			true,	"sticky bit unlink operations on idmapped mounts in user namespace",				},
> > +	{ sticky_bit_rename,						false,	"sticky bit rename operations on regular mounts",						},
> > +	{ sticky_bit_rename_idmapped_mounts,				true,	"sticky bit rename operations on idmapped mounts",						},
> > +	{ sticky_bit_rename_idmapped_mounts_in_userns,			true,	"sticky bit rename operations on idmapped mounts in user namespace",				},
> > +	{ symlink_regular_mounts,					false,	"symlink from regular mounts",									},
> > +	{ 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",							},
> >  };
> >  
> >  struct t_idmapped_mounts fscaps_in_ancestor_userns[] = {
> > -	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
> > +	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	true,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
> >  };
> >  
> >  struct t_idmapped_mounts t_nested_userns[] = {
> > -	{ nested_userns,						"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
> > +	{ nested_userns,						true,	"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
> >  };
> >  
> >  struct t_idmapped_mounts t_btrfs[] = {
> > -	{ btrfs_subvolumes_fsids_mapped,				"test subvolumes with mapped fsids",								},
> > -	{ btrfs_subvolumes_fsids_mapped_userns,				"test subvolumes with mapped fsids inside user namespace",					},
> > -	{ btrfs_subvolumes_fsids_mapped_user_subvol_rm_allowed,		"test subvolume deletion with user_subvol_rm_allowed mount option",				},
> > -	{ btrfs_subvolumes_fsids_mapped_userns_user_subvol_rm_allowed,	"test subvolume deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> > -	{ btrfs_subvolumes_fsids_unmapped,				"test subvolumes with unmapped fsids",								},
> > -	{ btrfs_subvolumes_fsids_unmapped_userns,			"test subvolumes with unmapped fsids inside user namespace",					},
> > -	{ btrfs_snapshots_fsids_mapped,					"test snapshots with mapped fsids",								},
> > -	{ btrfs_snapshots_fsids_mapped_userns,				"test snapshots with mapped fsids inside user namespace",					},
> > -	{ btrfs_snapshots_fsids_mapped_user_subvol_rm_allowed,		"test snapshots deletion with user_subvol_rm_allowed mount option",				},
> > -	{ btrfs_snapshots_fsids_mapped_userns_user_subvol_rm_allowed,	"test snapshots deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> > -	{ btrfs_snapshots_fsids_unmapped,				"test snapshots with unmapped fsids",								},
> > -	{ btrfs_snapshots_fsids_unmapped_userns,			"test snapshots with unmapped fsids inside user namespace",					},
> > -	{ btrfs_delete_by_spec_id,					"test subvolume deletion by spec id",								},
> > -	{ btrfs_subvolumes_setflags_fsids_mapped,			"test subvolume flags with mapped fsids",							},
> > -	{ btrfs_subvolumes_setflags_fsids_mapped_userns,		"test subvolume flags with mapped fsids inside user namespace",					},
> > -	{ btrfs_subvolumes_setflags_fsids_unmapped,			"test subvolume flags with unmapped fsids",							},
> > -	{ btrfs_subvolumes_setflags_fsids_unmapped_userns,		"test subvolume flags with unmapped fsids inside user namespace",				},
> > -	{ btrfs_snapshots_setflags_fsids_mapped,			"test snapshots flags with mapped fsids",							},
> > -	{ btrfs_snapshots_setflags_fsids_mapped_userns,			"test snapshots flags with mapped fsids inside user namespace",					},
> > -	{ btrfs_snapshots_setflags_fsids_unmapped,			"test snapshots flags with unmapped fsids",							},
> > -	{ btrfs_snapshots_setflags_fsids_unmapped_userns,		"test snapshots flags with unmapped fsids inside user namespace",				},
> > -	{ btrfs_subvolume_lookup_user,					"test unprivileged subvolume lookup",								},
> > +	{ btrfs_subvolumes_fsids_mapped,				true,	"test subvolumes with mapped fsids",								},
> > +	{ btrfs_subvolumes_fsids_mapped_userns,				true, 	"test subvolumes with mapped fsids inside user namespace",					},
> > +	{ btrfs_subvolumes_fsids_mapped_user_subvol_rm_allowed,		true, 	"test subvolume deletion with user_subvol_rm_allowed mount option",				},
> > +	{ btrfs_subvolumes_fsids_mapped_userns_user_subvol_rm_allowed,	true, 	"test subvolume deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> > +	{ btrfs_subvolumes_fsids_unmapped,				true, 	"test subvolumes with unmapped fsids",								},
> > +	{ btrfs_subvolumes_fsids_unmapped_userns,			true, 	"test subvolumes with unmapped fsids inside user namespace",					},
> > +	{ btrfs_snapshots_fsids_mapped,					true, 	"test snapshots with mapped fsids",								},
> > +	{ btrfs_snapshots_fsids_mapped_userns,				true, 	"test snapshots with mapped fsids inside user namespace",					},
> > +	{ btrfs_snapshots_fsids_mapped_user_subvol_rm_allowed,		true, 	"test snapshots deletion with user_subvol_rm_allowed mount option",				},
> > +	{ btrfs_snapshots_fsids_mapped_userns_user_subvol_rm_allowed,	true, 	"test snapshots deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> > +	{ btrfs_snapshots_fsids_unmapped,				true, 	"test snapshots with unmapped fsids",								},
> > +	{ btrfs_snapshots_fsids_unmapped_userns,			true, 	"test snapshots with unmapped fsids inside user namespace",					},
> > +	{ btrfs_delete_by_spec_id,					true, 	"test subvolume deletion by spec id",								},
> > +	{ btrfs_subvolumes_setflags_fsids_mapped,			true, 	"test subvolume flags with mapped fsids",							},
> > +	{ btrfs_subvolumes_setflags_fsids_mapped_userns,		true, 	"test subvolume flags with mapped fsids inside user namespace",					},
> > +	{ btrfs_subvolumes_setflags_fsids_unmapped,			true, 	"test subvolume flags with unmapped fsids",							},
> > +	{ btrfs_subvolumes_setflags_fsids_unmapped_userns,		true, 	"test subvolume flags with unmapped fsids inside user namespace",				},
> > +	{ btrfs_snapshots_setflags_fsids_mapped,			true, 	"test snapshots flags with mapped fsids",							},
> > +	{ btrfs_snapshots_setflags_fsids_mapped_userns,			true, 	"test snapshots flags with mapped fsids inside user namespace",					},
> > +	{ btrfs_snapshots_setflags_fsids_unmapped,			true, 	"test snapshots flags with unmapped fsids",							},
> > +	{ btrfs_snapshots_setflags_fsids_unmapped_userns,		true, 	"test snapshots flags with unmapped fsids inside user namespace",				},
> > +	{ btrfs_subvolume_lookup_user,					true, 	"test unprivileged subvolume lookup",								},
> >  };
> >  
> >  /* Test for commit 968219708108 ("fs: handle circular mappings correctly"). */
> >  struct t_idmapped_mounts t_setattr_fix_968219708108[] = {
> > -	{ setattr_fix_968219708108,					"test that setattr works correctly",								},
> > +	{ setattr_fix_968219708108,					true,	"test that setattr works correctly",								},
> >  };
> >  
> >  static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
> > @@ -13883,6 +13884,15 @@ static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
> >  		int ret;
> >  		pid_t pid;
> >  
> > +		/*
> > +		 * If the underlying filesystems does not support idmapped
> > +		 * mounts only run vfs generic tests.
> > +		 */
> > +		if (t->require_fs_allow_idmap && !t_fs_allow_idmap) {
> > +			log_debug("Skipping test %s", t->description);
> > +			continue;
> > +		}
> > +
> >  		test_setup();
> >  
> >  		pid = fork();
> > @@ -14011,13 +14021,15 @@ int main(int argc, char *argv[])
> >  	if (t_mnt_fd < 0)
> >  		die("failed to open %s", t_mountpoint_scratch);
> >  
> > -	/*
> > -	 * Caller just wants to know whether the filesystem we're on supports
> > -	 * idmapped mounts.
> > -	 */
> > +	t_fs_allow_idmap = fs_allow_idmap();
> >  	if (supported) {
> > -		if (!fs_allow_idmap())
> > +		/*
> > +		 * Caller just wants to know whether the filesystem we're on
> > +		 * supports idmapped mounts.
> > +		 */
> > +		if (!t_fs_allow_idmap)
> >  			exit(EXIT_FAILURE);
> > +
> >  		exit(EXIT_SUCCESS);
> >  	}
> >  
> > diff --git a/tests/generic/633 b/tests/generic/633
> > index 67501177..38280647 100755
> > --- a/tests/generic/633
> > +++ b/tests/generic/633
> > @@ -15,7 +15,6 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
> >  # real QA test starts here
> >  
> >  _supported_fs generic
> > -_require_idmapped_mounts
> 
> Removed by accident or is it intentional?

This is intentional. After this patch the binary - when asked to execute
the core test-suite - will detect whether the underlying filesystem
supports idmapped mounts. If it doesn't then it will skip all tests that
require idmapped mounts and therefore only execute the fully vfs generic
tests. I'll add a comment about it in the commit message.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] idmapped-mounts: always run generic vfs tests
  2022-01-18 13:37     ` Christian Brauner
@ 2022-01-18 13:40       ` Eryu Guan
  0 siblings, 0 replies; 7+ messages in thread
From: Eryu Guan @ 2022-01-18 13:40 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Eryu Guan, fstests, Christoph Hellwig, Seth Forshee, Eryu Guan

On Tue, Jan 18, 2022 at 02:37:11PM +0100, Christian Brauner wrote:
> On Sun, Jan 16, 2022 at 12:52:33PM +0800, Eryu Guan wrote:
> > On Thu, Jan 13, 2022 at 02:24:21PM +0100, Christian Brauner wrote:
> > > Make it possible to always run all the tests in the testsuite that don't
> > > require idmapped mounts. Now all filesystems can benefit from the
> > > generic vfs tests that we currently implement. This means setgid
> > > inheritance and other tests will be run for all filesystems not matter
> > > if they support idmapped mounts or not.
> > > 
> > > Cc: Seth Forshee <sforshee@digitalocean.com>
> > > Cc: Eryu Guan <guaneryu@gmail.com>
> > > Cc: Christoph Hellwig <hch@lst.de>
> > > Cc: fstests@vger.kernel.org
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > ---
> > >  src/idmapped-mounts/idmapped-mounts.c | 184 ++++++++++++++------------
> > >  tests/generic/633                     |   1 -
> > >  2 files changed, 98 insertions(+), 87 deletions(-)
> > > 
> > > diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
> > > index a78a901f..5f304108 100644
> > > --- a/src/idmapped-mounts/idmapped-mounts.c
> > > +++ b/src/idmapped-mounts/idmapped-mounts.c
> > > @@ -125,6 +125,9 @@ int t_dir1_fd;
> > >  /* temporary buffer */
> > >  char t_buf[PATH_MAX];
> > >  
> > > +/* whether the underlying filesystem supports idmapped mounts */
> > > +bool t_fs_allow_idmap;
> > > +
> > >  static void stash_overflowuid(void)
> > >  {
> > >  	int fd;
> > > @@ -2867,10 +2870,7 @@ out:
> > >  static int fscaps(void)
> > >  {
> > >  	int fret = -1;
> > > -	int file1_fd = -EBADF;
> > > -	struct mount_attr attr = {
> > > -		.attr_set = MOUNT_ATTR_IDMAP,
> > > -	};
> > > +	int file1_fd = -EBADF, fd_userns = -EBADF;
> > >  	pid_t pid;
> > >  
> > >  	file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, 0644);
> > > @@ -2884,8 +2884,8 @@ static int fscaps(void)
> > >  		return 0;
> > >  
> > >  	/* Changing mount properties on a detached mount. */
> > > -	attr.userns_fd	= get_userns_fd(0, 10000, 10000);
> > > -	if (attr.userns_fd < 0) {
> > > +	fd_userns = get_userns_fd(0, 10000, 10000);
> > > +	if (fd_userns < 0) {
> > >  		log_stderr("failure: get_userns_fd");
> > >  		goto out;
> > >  	}
> > > @@ -2901,7 +2901,7 @@ static int fscaps(void)
> > >  		goto out;
> > >  	}
> > >  	if (pid == 0) {
> > > -		if (!switch_userns(attr.userns_fd, 0, 0, false))
> > > +		if (!switch_userns(fd_userns, 0, 0, false))
> > >  			die("failure: switch_userns");
> > >  
> > >  		/*
> > > @@ -2949,7 +2949,7 @@ static int fscaps(void)
> > >  		goto out;
> > >  	}
> > >  	if (pid == 0) {
> > > -		if (!switch_userns(attr.userns_fd, 0, 0, false))
> > > +		if (!switch_userns(fd_userns, 0, 0, false))
> > >  			die("failure: switch_userns");
> > >  
> > >  		if (!expected_dummy_vfs_caps_uid(file1_fd, 0))
> > > @@ -2977,8 +2977,8 @@ static int fscaps(void)
> > >  	fret = 0;
> > >  	log_debug("Ran test");
> > >  out:
> > > -	safe_close(attr.userns_fd);
> > >  	safe_close(file1_fd);
> > > +	safe_close(fd_userns);
> > >  
> > >  	return fret;
> > >  }
> > > @@ -13783,95 +13783,96 @@ static const struct option longopts[] = {
> > >  
> > >  struct t_idmapped_mounts {
> > >  	int (*test)(void);
> > > +	bool require_fs_allow_idmap;
> > >  	const char *description;
> > >  } basic_suite[] = {
> > > -	{ acls,								"posix acls on regular mounts",									},
> > > -	{ create_in_userns,						"create operations in user namespace",								},
> > > -	{ device_node_in_userns,					"device node in user namespace",								},
> > > -	{ expected_uid_gid_idmapped_mounts,				"expected ownership on idmapped mounts",							},
> > > -	{ fscaps,							"fscaps on regular mounts",									},
> > > -	{ fscaps_idmapped_mounts,					"fscaps on idmapped mounts",									},
> > > -	{ fscaps_idmapped_mounts_in_userns,				"fscaps on idmapped mounts in user namespace",							},
> > > -	{ fscaps_idmapped_mounts_in_userns_separate_userns,		"fscaps on idmapped mounts in user namespace with different id mappings",			},
> > > -	{ fsids_mapped,							"mapped fsids",											},
> > > -	{ fsids_unmapped,						"unmapped fsids",										},
> > > -	{ hardlink_crossing_mounts,					"cross mount hardlink",										},
> > > -	{ hardlink_crossing_idmapped_mounts,				"cross idmapped mount hardlink",								},
> > > -	{ hardlink_from_idmapped_mount,					"hardlinks from idmapped mounts",								},
> > > -	{ hardlink_from_idmapped_mount_in_userns,			"hardlinks from idmapped mounts in user namespace",						},
> > > +	{ acls,								true,	"posix acls on regular mounts",									},
> > > +	{ create_in_userns,						true,	"create operations in user namespace",								},
> > > +	{ device_node_in_userns,					true,	"device node in user namespace",								},
> > > +	{ expected_uid_gid_idmapped_mounts,				true,	"expected ownership on idmapped mounts",							},
> > > +	{ fscaps,							false,	"fscaps on regular mounts",									},
> > > +	{ fscaps_idmapped_mounts,					true,	"fscaps on idmapped mounts",									},
> > > +	{ fscaps_idmapped_mounts_in_userns,				true,	"fscaps on idmapped mounts in user namespace",							},
> > > +	{ fscaps_idmapped_mounts_in_userns_separate_userns,		true,	"fscaps on idmapped mounts in user namespace with different id mappings",			},
> > > +	{ fsids_mapped,							true,	"mapped fsids",											},
> > > +	{ fsids_unmapped,						true,	"unmapped fsids",										},
> > > +	{ hardlink_crossing_mounts,					false,	"cross mount hardlink",										},
> > > +	{ hardlink_crossing_idmapped_mounts,				true,	"cross idmapped mount hardlink",								},
> > > +	{ hardlink_from_idmapped_mount,					true,	"hardlinks from idmapped mounts",								},
> > > +	{ hardlink_from_idmapped_mount_in_userns,			true,	"hardlinks from idmapped mounts in user namespace",						},
> > >  #ifdef HAVE_LIBURING_H
> > > -	{ io_uring,							"io_uring",											},
> > > -	{ io_uring_userns,						"io_uring in user namespace",									},
> > > -	{ io_uring_idmapped,						"io_uring from idmapped mounts",								},
> > > -	{ io_uring_idmapped_userns,					"io_uring from idmapped mounts in user namespace",						},
> > > -	{ io_uring_idmapped_unmapped,					"io_uring from idmapped mounts with unmapped ids",						},
> > > -	{ io_uring_idmapped_unmapped_userns,				"io_uring from idmapped mounts with unmapped ids in user namespace",				},
> > > +	{ io_uring,							false,	"io_uring",											},
> > > +	{ io_uring_userns,						false,	"io_uring in user namespace",									},
> > > +	{ io_uring_idmapped,						true,	"io_uring from idmapped mounts",								},
> > > +	{ io_uring_idmapped_userns,					true,	"io_uring from idmapped mounts in user namespace",						},
> > > +	{ io_uring_idmapped_unmapped,					true,	"io_uring from idmapped mounts with unmapped ids",						},
> > > +	{ io_uring_idmapped_unmapped_userns,				true,	"io_uring from idmapped mounts with unmapped ids in user namespace",				},
> > >  #endif
> > > -	{ protected_symlinks,						"following protected symlinks on regular mounts",						},
> > > -	{ protected_symlinks_idmapped_mounts,				"following protected symlinks on idmapped mounts",						},
> > > -	{ protected_symlinks_idmapped_mounts_in_userns,			"following protected symlinks on idmapped mounts in user namespace",				},
> > > -	{ rename_crossing_mounts,					"cross mount rename",										},
> > > -	{ rename_crossing_idmapped_mounts,				"cross idmapped mount rename",									},
> > > -	{ rename_from_idmapped_mount,					"rename from idmapped mounts",									},
> > > -	{ rename_from_idmapped_mount_in_userns,				"rename from idmapped mounts in user namespace",						},
> > > -	{ setattr_truncate,						"setattr truncate",										},
> > > -	{ setattr_truncate_idmapped,					"setattr truncate on idmapped mounts",								},
> > > -	{ setattr_truncate_idmapped_in_userns,				"setattr truncate on idmapped mounts in user namespace",					},
> > > -	{ setgid_create,						"create operations in directories with setgid bit set",						},
> > > -	{ setgid_create_idmapped,					"create operations in directories with setgid bit set on idmapped mounts",			},
> > > -	{ setgid_create_idmapped_in_userns,				"create operations in directories with setgid bit set on idmapped mounts in user namespace",	},
> > > -	{ setid_binaries,						"setid binaries on regular mounts",								},
> > > -	{ setid_binaries_idmapped_mounts,				"setid binaries on idmapped mounts",								},
> > > -	{ setid_binaries_idmapped_mounts_in_userns,			"setid binaries on idmapped mounts in user namespace",						},
> > > -	{ setid_binaries_idmapped_mounts_in_userns_separate_userns,	"setid binaries on idmapped mounts in user namespace with different id mappings",		},
> > > -	{ sticky_bit_unlink,						"sticky bit unlink operations on regular mounts",						},
> > > -	{ sticky_bit_unlink_idmapped_mounts,				"sticky bit unlink operations on idmapped mounts",						},
> > > -	{ sticky_bit_unlink_idmapped_mounts_in_userns,			"sticky bit unlink operations on idmapped mounts in user namespace",				},
> > > -	{ sticky_bit_rename,						"sticky bit rename operations on regular mounts",						},
> > > -	{ sticky_bit_rename_idmapped_mounts,				"sticky bit rename operations on idmapped mounts",						},
> > > -	{ sticky_bit_rename_idmapped_mounts_in_userns,			"sticky bit rename operations on idmapped mounts in user namespace",				},
> > > -	{ symlink_regular_mounts,					"symlink from regular mounts",									},
> > > -	{ symlink_idmapped_mounts,					"symlink from idmapped mounts",									},
> > > -	{ symlink_idmapped_mounts_in_userns,				"symlink from idmapped mounts in user namespace",						},
> > > -	{ threaded_idmapped_mount_interactions,				"threaded operations on idmapped mounts",							},
> > > +	{ protected_symlinks,						false,	"following protected symlinks on regular mounts",						},
> > > +	{ protected_symlinks_idmapped_mounts,				true,	"following protected symlinks on idmapped mounts",						},
> > > +	{ protected_symlinks_idmapped_mounts_in_userns,			true,	"following protected symlinks on idmapped mounts in user namespace",				},
> > > +	{ rename_crossing_mounts,					false,	"cross mount rename",										},
> > > +	{ rename_crossing_idmapped_mounts,				true,	"cross idmapped mount rename",									},
> > > +	{ rename_from_idmapped_mount,					true,	"rename from idmapped mounts",									},
> > > +	{ rename_from_idmapped_mount_in_userns,				true,	"rename from idmapped mounts in user namespace",						},
> > > +	{ setattr_truncate,						false,	"setattr truncate",										},
> > > +	{ setattr_truncate_idmapped,					true,	"setattr truncate on idmapped mounts",								},
> > > +	{ setattr_truncate_idmapped_in_userns,				true,	"setattr truncate on idmapped mounts in user namespace",					},
> > > +	{ setgid_create,						false,	"create operations in directories with setgid bit set",						},
> > > +	{ setgid_create_idmapped,					true,	"create operations in directories with setgid bit set on idmapped mounts",			},
> > > +	{ setgid_create_idmapped_in_userns,				true,	"create operations in directories with setgid bit set on idmapped mounts in user namespace",	},
> > > +	{ setid_binaries,						false,	"setid binaries on regular mounts",								},
> > > +	{ setid_binaries_idmapped_mounts,				true,	"setid binaries on idmapped mounts",								},
> > > +	{ setid_binaries_idmapped_mounts_in_userns,			true,	"setid binaries on idmapped mounts in user namespace",						},
> > > +	{ setid_binaries_idmapped_mounts_in_userns_separate_userns,	true,	"setid binaries on idmapped mounts in user namespace with different id mappings",		},
> > > +	{ sticky_bit_unlink,						false,	"sticky bit unlink operations on regular mounts",						},
> > > +	{ sticky_bit_unlink_idmapped_mounts,				true,	"sticky bit unlink operations on idmapped mounts",						},
> > > +	{ sticky_bit_unlink_idmapped_mounts_in_userns,			true,	"sticky bit unlink operations on idmapped mounts in user namespace",				},
> > > +	{ sticky_bit_rename,						false,	"sticky bit rename operations on regular mounts",						},
> > > +	{ sticky_bit_rename_idmapped_mounts,				true,	"sticky bit rename operations on idmapped mounts",						},
> > > +	{ sticky_bit_rename_idmapped_mounts_in_userns,			true,	"sticky bit rename operations on idmapped mounts in user namespace",				},
> > > +	{ symlink_regular_mounts,					false,	"symlink from regular mounts",									},
> > > +	{ 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",							},
> > >  };
> > >  
> > >  struct t_idmapped_mounts fscaps_in_ancestor_userns[] = {
> > > -	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
> > > +	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	true,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
> > >  };
> > >  
> > >  struct t_idmapped_mounts t_nested_userns[] = {
> > > -	{ nested_userns,						"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
> > > +	{ nested_userns,						true,	"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
> > >  };
> > >  
> > >  struct t_idmapped_mounts t_btrfs[] = {
> > > -	{ btrfs_subvolumes_fsids_mapped,				"test subvolumes with mapped fsids",								},
> > > -	{ btrfs_subvolumes_fsids_mapped_userns,				"test subvolumes with mapped fsids inside user namespace",					},
> > > -	{ btrfs_subvolumes_fsids_mapped_user_subvol_rm_allowed,		"test subvolume deletion with user_subvol_rm_allowed mount option",				},
> > > -	{ btrfs_subvolumes_fsids_mapped_userns_user_subvol_rm_allowed,	"test subvolume deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> > > -	{ btrfs_subvolumes_fsids_unmapped,				"test subvolumes with unmapped fsids",								},
> > > -	{ btrfs_subvolumes_fsids_unmapped_userns,			"test subvolumes with unmapped fsids inside user namespace",					},
> > > -	{ btrfs_snapshots_fsids_mapped,					"test snapshots with mapped fsids",								},
> > > -	{ btrfs_snapshots_fsids_mapped_userns,				"test snapshots with mapped fsids inside user namespace",					},
> > > -	{ btrfs_snapshots_fsids_mapped_user_subvol_rm_allowed,		"test snapshots deletion with user_subvol_rm_allowed mount option",				},
> > > -	{ btrfs_snapshots_fsids_mapped_userns_user_subvol_rm_allowed,	"test snapshots deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> > > -	{ btrfs_snapshots_fsids_unmapped,				"test snapshots with unmapped fsids",								},
> > > -	{ btrfs_snapshots_fsids_unmapped_userns,			"test snapshots with unmapped fsids inside user namespace",					},
> > > -	{ btrfs_delete_by_spec_id,					"test subvolume deletion by spec id",								},
> > > -	{ btrfs_subvolumes_setflags_fsids_mapped,			"test subvolume flags with mapped fsids",							},
> > > -	{ btrfs_subvolumes_setflags_fsids_mapped_userns,		"test subvolume flags with mapped fsids inside user namespace",					},
> > > -	{ btrfs_subvolumes_setflags_fsids_unmapped,			"test subvolume flags with unmapped fsids",							},
> > > -	{ btrfs_subvolumes_setflags_fsids_unmapped_userns,		"test subvolume flags with unmapped fsids inside user namespace",				},
> > > -	{ btrfs_snapshots_setflags_fsids_mapped,			"test snapshots flags with mapped fsids",							},
> > > -	{ btrfs_snapshots_setflags_fsids_mapped_userns,			"test snapshots flags with mapped fsids inside user namespace",					},
> > > -	{ btrfs_snapshots_setflags_fsids_unmapped,			"test snapshots flags with unmapped fsids",							},
> > > -	{ btrfs_snapshots_setflags_fsids_unmapped_userns,		"test snapshots flags with unmapped fsids inside user namespace",				},
> > > -	{ btrfs_subvolume_lookup_user,					"test unprivileged subvolume lookup",								},
> > > +	{ btrfs_subvolumes_fsids_mapped,				true,	"test subvolumes with mapped fsids",								},
> > > +	{ btrfs_subvolumes_fsids_mapped_userns,				true, 	"test subvolumes with mapped fsids inside user namespace",					},
> > > +	{ btrfs_subvolumes_fsids_mapped_user_subvol_rm_allowed,		true, 	"test subvolume deletion with user_subvol_rm_allowed mount option",				},
> > > +	{ btrfs_subvolumes_fsids_mapped_userns_user_subvol_rm_allowed,	true, 	"test subvolume deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> > > +	{ btrfs_subvolumes_fsids_unmapped,				true, 	"test subvolumes with unmapped fsids",								},
> > > +	{ btrfs_subvolumes_fsids_unmapped_userns,			true, 	"test subvolumes with unmapped fsids inside user namespace",					},
> > > +	{ btrfs_snapshots_fsids_mapped,					true, 	"test snapshots with mapped fsids",								},
> > > +	{ btrfs_snapshots_fsids_mapped_userns,				true, 	"test snapshots with mapped fsids inside user namespace",					},
> > > +	{ btrfs_snapshots_fsids_mapped_user_subvol_rm_allowed,		true, 	"test snapshots deletion with user_subvol_rm_allowed mount option",				},
> > > +	{ btrfs_snapshots_fsids_mapped_userns_user_subvol_rm_allowed,	true, 	"test snapshots deletion with user_subvol_rm_allowed mount option inside user namespace",	},
> > > +	{ btrfs_snapshots_fsids_unmapped,				true, 	"test snapshots with unmapped fsids",								},
> > > +	{ btrfs_snapshots_fsids_unmapped_userns,			true, 	"test snapshots with unmapped fsids inside user namespace",					},
> > > +	{ btrfs_delete_by_spec_id,					true, 	"test subvolume deletion by spec id",								},
> > > +	{ btrfs_subvolumes_setflags_fsids_mapped,			true, 	"test subvolume flags with mapped fsids",							},
> > > +	{ btrfs_subvolumes_setflags_fsids_mapped_userns,		true, 	"test subvolume flags with mapped fsids inside user namespace",					},
> > > +	{ btrfs_subvolumes_setflags_fsids_unmapped,			true, 	"test subvolume flags with unmapped fsids",							},
> > > +	{ btrfs_subvolumes_setflags_fsids_unmapped_userns,		true, 	"test subvolume flags with unmapped fsids inside user namespace",				},
> > > +	{ btrfs_snapshots_setflags_fsids_mapped,			true, 	"test snapshots flags with mapped fsids",							},
> > > +	{ btrfs_snapshots_setflags_fsids_mapped_userns,			true, 	"test snapshots flags with mapped fsids inside user namespace",					},
> > > +	{ btrfs_snapshots_setflags_fsids_unmapped,			true, 	"test snapshots flags with unmapped fsids",							},
> > > +	{ btrfs_snapshots_setflags_fsids_unmapped_userns,		true, 	"test snapshots flags with unmapped fsids inside user namespace",				},
> > > +	{ btrfs_subvolume_lookup_user,					true, 	"test unprivileged subvolume lookup",								},
> > >  };
> > >  
> > >  /* Test for commit 968219708108 ("fs: handle circular mappings correctly"). */
> > >  struct t_idmapped_mounts t_setattr_fix_968219708108[] = {
> > > -	{ setattr_fix_968219708108,					"test that setattr works correctly",								},
> > > +	{ setattr_fix_968219708108,					true,	"test that setattr works correctly",								},
> > >  };
> > >  
> > >  static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
> > > @@ -13883,6 +13884,15 @@ static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
> > >  		int ret;
> > >  		pid_t pid;
> > >  
> > > +		/*
> > > +		 * If the underlying filesystems does not support idmapped
> > > +		 * mounts only run vfs generic tests.
> > > +		 */
> > > +		if (t->require_fs_allow_idmap && !t_fs_allow_idmap) {
> > > +			log_debug("Skipping test %s", t->description);
> > > +			continue;
> > > +		}
> > > +
> > >  		test_setup();
> > >  
> > >  		pid = fork();
> > > @@ -14011,13 +14021,15 @@ int main(int argc, char *argv[])
> > >  	if (t_mnt_fd < 0)
> > >  		die("failed to open %s", t_mountpoint_scratch);
> > >  
> > > -	/*
> > > -	 * Caller just wants to know whether the filesystem we're on supports
> > > -	 * idmapped mounts.
> > > -	 */
> > > +	t_fs_allow_idmap = fs_allow_idmap();
> > >  	if (supported) {
> > > -		if (!fs_allow_idmap())
> > > +		/*
> > > +		 * Caller just wants to know whether the filesystem we're on
> > > +		 * supports idmapped mounts.
> > > +		 */
> > > +		if (!t_fs_allow_idmap)
> > >  			exit(EXIT_FAILURE);
> > > +
> > >  		exit(EXIT_SUCCESS);
> > >  	}
> > >  
> > > diff --git a/tests/generic/633 b/tests/generic/633
> > > index 67501177..38280647 100755
> > > --- a/tests/generic/633
> > > +++ b/tests/generic/633
> > > @@ -15,7 +15,6 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
> > >  # real QA test starts here
> > >  
> > >  _supported_fs generic
> > > -_require_idmapped_mounts
> > 
> > Removed by accident or is it intentional?
> 
> This is intentional. After this patch the binary - when asked to execute
> the core test-suite - will detect whether the underlying filesystem
> supports idmapped mounts. If it doesn't then it will skip all tests that
> require idmapped mounts and therefore only execute the fully vfs generic
> tests. I'll add a comment about it in the commit message.

But _require_idmapped_mounts also requires the test binary exists,
without this rule test fails if we forget to compile idmapped-mount test
binary in src dir.

Then we need this I think

_require_test_program idmapped-mounts/idmapped-mounts

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-01-18 13:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 13:24 [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper Christian Brauner
2022-01-13 13:24 ` [PATCH 2/2] idmapped-mounts: always run generic vfs tests Christian Brauner
2022-01-16  4:52   ` Eryu Guan
2022-01-18 13:37     ` Christian Brauner
2022-01-18 13:40       ` Eryu Guan
2022-01-16  4:51 ` [PATCH 1/2] idmapped-mounts: add fs_allow_idmap() helper Eryu Guan
2022-01-18 13:33   ` Christian Brauner

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.