Merge patch series "mount: a few gnarly fixes"

Christian Brauner <brauner@kernel.org> says:

A short while ago I received a bug report about problematic behavior in
the mount code and that let me to review and model a few corner cases
including reference counting. The result is the following set of fixes:

- Ensure that copies of unbindable mounts remain unbindable
- Ensure that MOVE_MOUNT_SET_GROUP doesn't operate on unbindable mounts
  so we don't create an undefined state where a mount is both unbindable
  and a slave mount.
- Ensure that we don't silently unmount busy mounts. This is nasty
  because it's an old bug from v4.13 where calculation of whether a
  synchronous (non-MNT_DETACH) umount would succeed drifted apart from
  the actual codepath that did the unmount.
- Ensure that we don't silently unmount busy mounts. Same class,
  different mechanism. There's a race between the reference count check
  in propagate_mount_busy() (summing up per-cpu counters) and mntget().

Fixes and test for all. No test for the mntget() race because that needs
aritifical delays.

And fwiw, I think I another vomitorious series in the pipes.

* patches from https://patch.msgid.link/20260923-work-mount-fixes-v1-0-f424cf8d3242@kernel.org:
  docs: update the unmount propagation rule
  fs: don't let a migrating task hide its reference from do_umount()
  selftests/filesystems: check that a busy propagated copy blocks a synchronous umount
  fs: don't silently unmount busy mounts
  selftests/move_mount_set_group: check that an unbindable target is refused
  mount: refuse MOVE_MOUNT_SET_GROUP on an unbindable mount
  selftests/filesystems: check that a copied mount namespace keeps unbindable
  mount: keep a copied mount unbindable

Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-0-f424cf8d3242@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Documentation/filesystems/sharedsubtree.rst

@@ -564,8 +564,8 @@ f) Unmount semantics
    where 'A' is a mount mounted on mount 'B' at dentry 'b'.
 
    If mount 'B' is shared, then all most-recently-mounted mounts at dentry
-   'b' on mounts that receive propagation from mount 'B' and does not have
-   sub-mounts within them are unmounted.
+   'b' on mounts that receive propagation from mount 'B' are unmounted as
+   well, if every mount below them is also unmounted.
 
    Example: Let's say 'B1', 'B2', 'B3' are shared mounts that propagate to
    each other.
@@ -584,10 +584,18 @@ f) Unmount semantics
 
    So all 'C1', 'C2' and 'C3' should be unmounted.
 
-   If any of 'C2' or 'C3' has some child mounts, then that mount is not
-   unmounted, but all other mounts are unmounted. However if 'C1' is told
-   to be unmounted and 'C1' has some sub-mounts, the umount operation is
-   failed entirely.
+   If any of 'C2' or 'C3' has a child mount that cannot be unmounted
+   then that mount is not unmounted. But all other mounts are unmounted.
+   A child mount that is itself unmounted by the same unmount
+   propagation does not keep its parent mounted. However if 'C1' is
+   supposed to be unmounted and 'C1' has some sub-mounts, the unmount
+   fails.
+
+   A lazy umount (MNT_DETACH) takes a whole tree. Every mount of the
+   tree then propagates its unmount from its own parent as described
+   above, so the mounts that receive propagation lose the corresponding
+   trees as well. Documentation/filesystems/propagate_umount.txt has the
+   precise rules, including the ones for locked mounts.
 
 g) Clone Namespace
 

fs/mount.h

@@ -33,7 +33,8 @@ struct mnt_namespace {
 } __randomize_layout;
 
 struct mnt_pcp {
-	int mnt_count;
+	unsigned int mnt_gets;
+	unsigned int mnt_puts;
 	int mnt_writers;
 };
 

fs/namespace.c

@@ -249,16 +249,24 @@ void mnt_release_group_id(struct mount *mnt)
 	mnt->mnt_group_id = 0;
 }
 
-/*
- * vfsmount lock must be held for read
- */
-static inline void mnt_add_count(struct mount *mnt, int n)
+static inline void mnt_inc_count(struct mount *mnt)
 {
 #ifdef CONFIG_SMP
-	this_cpu_add(mnt->mnt_pcp->mnt_count, n);
+	this_cpu_inc(mnt->mnt_pcp->mnt_gets);
 #else
 	preempt_disable();
-	mnt->mnt_count += n;
+	mnt->mnt_count++;
+	preempt_enable();
+#endif
+}
+
+static inline void mnt_dec_count(struct mount *mnt)
+{
+#ifdef CONFIG_SMP
+	this_cpu_inc(mnt->mnt_pcp->mnt_puts);
+#else
+	preempt_disable();
+	mnt->mnt_count--;
 	preempt_enable();
 #endif
 }
@@ -269,14 +277,17 @@ static inline void mnt_add_count(struct mount *mnt, int n)
 int mnt_get_count(struct mount *mnt)
 {
 #ifdef CONFIG_SMP
-	int count = 0;
+	unsigned int gets = 0, puts = 0;
 	int cpu;
 
-	for_each_possible_cpu(cpu) {
-		count += per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_count;
-	}
+	/* puts first, so a put counted here has its get counted below */
+	for_each_possible_cpu(cpu)
+		puts += per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_puts;
+	smp_mb();	/* pairs with the smp_wmb() in mntput_no_expire() */
+	for_each_possible_cpu(cpu)
+		gets += per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_gets;
 
-	return count;
+	return gets - puts;
 #else
 	return mnt->mnt_count;
 #endif
@@ -305,7 +316,7 @@ static struct mount *alloc_vfsmnt(const char *name)
 		if (!mnt->mnt_pcp)
 			goto out_free_devname;
 
-		this_cpu_add(mnt->mnt_pcp->mnt_count, 1);
+		this_cpu_inc(mnt->mnt_pcp->mnt_gets);
 #else
 		mnt->mnt_count = 1;
 		mnt->mnt_writers = 0;
@@ -746,13 +757,13 @@ int __legitimize_mnt(struct vfsmount *bastard, unsigned seq)
 	if (bastard == NULL)
 		return 0;
 	mnt = real_mount(bastard);
-	mnt_add_count(mnt, 1);
-	smp_mb();		// see mntput_no_expire() and do_umount()
+	mnt_inc_count(mnt);
+	smp_mb();	/* see mntput_no_expire_slowpath() and do_umount() */
 	if (likely(!read_seqretry(&mount_lock, seq)))
 		return 0;
 	lock_mount_hash();
 	if (unlikely(bastard->mnt_flags & (MNT_SYNC_UMOUNT | MNT_DOOMED))) {
-		mnt_add_count(mnt, -1);
+		mnt_dec_count(mnt);
 		unlock_mount_hash();
 		return 1;
 	}
@@ -1254,6 +1265,7 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
 
 	mnt->mnt.mnt_flags = READ_ONCE(old->mnt.mnt_flags) &
 			     ~MNT_INTERNAL_FLAGS;
+	mnt->mnt_t_flags = old->mnt_t_flags & T_UNBINDABLE;
 
 	if (flag & (CL_SLAVE | CL_PRIVATE))
 		mnt->mnt_group_id = 0; /* not a peer of original */
@@ -1347,7 +1359,7 @@ static void noinline mntput_no_expire_slowpath(struct mount *mnt)
 	 * mount_lock, we'll see their refcount increment here.
 	 */
 	smp_mb();
-	mnt_add_count(mnt, -1);
+	mnt_dec_count(mnt);
 	count = mnt_get_count(mnt);
 	if (count != 0) {
 		WARN_ON(count < 0);
@@ -1404,7 +1416,8 @@ static void mntput_no_expire(struct mount *mnt)
 		 * non-NULL under rcu_read_lock(), the reference
 		 * we are dropping is not the final one.
 		 */
-		mnt_add_count(mnt, -1);
+		smp_wmb();	/* pairs with the smp_mb() in mnt_get_count() */
+		mnt_dec_count(mnt);
 		rcu_read_unlock();
 		return;
 	}
@@ -1426,7 +1439,7 @@ EXPORT_SYMBOL(mntput);
 struct vfsmount *mntget(struct vfsmount *mnt)
 {
 	if (mnt)
-		mnt_add_count(real_mount(mnt), 1);
+		mnt_inc_count(real_mount(mnt));
 	return mnt;
 }
 EXPORT_SYMBOL(mntget);
@@ -3467,7 +3480,7 @@ static int do_set_group(const struct path *from_path, const struct path *to_path
 		return -EINVAL;
 
 	/* Setting sharing groups is only allowed on private mounts */
-	if (IS_MNT_SHARED(to) || IS_MNT_SLAVE(to))
+	if (IS_MNT_SHARED(to) || IS_MNT_SLAVE(to) || IS_MNT_UNBINDABLE(to))
 		return -EINVAL;
 
 	/* From should not be private */

fs/pnode.c

@@ -410,19 +410,99 @@ bool propagation_would_overmount(const struct mount *from,
 	return false;
 }
 
+/* Does @m receive propagation from @parent? */
+static bool receives_from(struct mount *m, struct mount *parent)
+{
+	if (m == parent)
+		return false;
+	for (; m; m = m->mnt_master)
+		if (m == parent || peers(m, parent))
+			return true;
+	return false;
+}
+
+/*
+ * Does @m receive propagation from the victim's parent as well? If so, then
+ * the mount at the victim's mountpoint inside of @m is a umount candidate as
+ * well. So it's the next candidate in the chain. Otherwise the chain ends at
+ * @m.
+ */
+static struct mount *next_candidate(struct mount *m, struct mount *victim)
+{
+	if (!receives_from(m, victim->mnt_parent))
+		return NULL;
+	return __lookup_mnt(&m->mnt, victim->mnt_mountpoint);
+}
+
+/*
+ * Would propagate_umount() pull out a mount of the chain of candidates that
+ * starts at @c, and does that mount have references beyond its own?
+ *
+ * This mirrors how trim_one(), trim_ancestors() and handle_locked() handle a
+ * synchronous umount:
+ *
+ * - single victim
+ * - without children
+ * - with MNT_LOCKED already cleared on every candidate by propagate_mount_unlock()
+ *
+ * A copy of the victim gets unmounted when each of its children is
+ * the next candidate in the chain or its overmount, unless the next
+ * unmount candidate is not its overmount and some unmount candidate further
+ * down has a child outside the chain.  Keep this in sync with
+ * Documentation/filesystems/propagate_umount.txt.
+ */
+static bool chain_busy(struct mount *c, struct mount *victim)
+{
+	struct mount *m, *n, *next, *deepest = NULL;
+	bool above;
+
+	/* the deepest candidate with a child outside the chain */
+	for (m = c; m; m = next) {
+		next = next_candidate(m, victim);
+		list_for_each_entry(n, &m->mnt_mounts, mnt_child) {
+			if (n != next && n != victim) {
+				deepest = m;
+				break;
+			}
+		}
+	}
+
+	above = deepest != NULL;	/* @deepest is at or below @m */
+	for (m = c; m; m = next) {
+		bool goes = true;
+
+		next = next_candidate(m, victim);
+		list_for_each_entry(n, &m->mnt_mounts, mnt_child) {
+			if (n != next && n != m->overmount && n != victim) {
+				goes = false;
+				break;
+			}
+		}
+		if (goes && next && next != m->overmount && above && m != deepest)
+			goes = false;
+		if (m == deepest)
+			above = false;
+		if (goes && do_refcount_check(m, 1))
+			return true;
+	}
+	return false;
+}
+
 /*
  * check if the mount 'mnt' can be unmounted successfully.
  * @mnt: the mount to be checked for unmount
  * NOTE: unmounting 'mnt' would naturally propagate to all
  * other mounts its parent propagates to.
- * Check if any of these mounts that **do not have submounts**
- * have more references than 'refcnt'. If so return busy.
+ * Check if any of the mounts that propagate_umount() would pull out
+ * along with it have more references than their own. If so return busy.
  *
  * vfsmount lock must be held for write
  */
 int propagate_mount_busy(struct mount *mnt, int refcnt)
 {
 	struct mount *parent = mnt->mnt_parent;
+	struct dentry *mp = mnt->mnt_mountpoint;
+	struct mount *m;
 
 	/*
 	 * quickly check if the current mount can be unmounted.
@@ -435,24 +515,16 @@ int propagate_mount_busy(struct mount *mnt, int refcnt)
 	if (mnt == parent)
 		return 0;
 
-	for (struct mount *m = propagation_next(parent, parent); m;
-	     		m = propagation_next(m, parent)) {
-		struct list_head *head;
-		struct mount *child = __lookup_mnt(&m->mnt, mnt->mnt_mountpoint);
+	/* the candidates are the mounts at @mp below the receivers */
+	for (m = propagation_next(parent, parent); m;
+	     m = propagation_next(m, parent)) {
+		struct mount *c = __lookup_mnt(&m->mnt, mp);
 
-		if (!child)
+		/* each chain once, from its top: skip receivers that are candidates */
+		if (!c || (mnt_has_parent(m) && m->mnt_mountpoint == mp &&
+			   receives_from(m->mnt_parent, parent)))
 			continue;
-
-		head = &child->mnt_mounts;
-		if (!list_empty(head)) {
-			/*
-			 * a mount that covers child completely wouldn't prevent
-			 * it being pulled out; any other would.
-			 */
-			if (!list_is_singular(head) || !child->overmount)
-				continue;
-		}
-		if (do_refcount_check(child, 1))
+		if (chain_busy(c, mnt))
 			return 1;
 	}
 	return 0;

tools/testing/selftests/Makefile

@@ -50,6 +50,8 @@ TARGETS += filesystems/empty_mntns
 TARGETS += filesystems/fsmount_ns
 TARGETS += filesystems/fscontext_ns
 TARGETS += filesystems/xattr
+TARGETS += filesystems/mntns_unbindable
+TARGETS += filesystems/umount_propagation
 TARGETS += firmware
 TARGETS += fpu
 TARGETS += ftrace

tools/testing/selftests/filesystems/mntns_unbindable/Makefile

@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+TEST_GEN_PROGS := mntns_unbindable_test
+
+CFLAGS += -Wall -O2 -g $(KHDR_INCLUDES)
+
+include ../../lib.mk

tools/testing/selftests/filesystems/mntns_unbindable/mntns_unbindable_test.c

@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * An unbindable mount stays unbindable in a cloned mount namespace.
+ */
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "../../kselftest_harness.h"
+
+#ifndef OPEN_TREE_CLONE
+#define OPEN_TREE_CLONE		1
+#endif
+#ifndef OPEN_TREE_CLOEXEC
+#define OPEN_TREE_CLOEXEC	O_CLOEXEC
+#endif
+
+static int sys_open_tree(int dfd, const char *filename, unsigned int flags)
+{
+	return syscall(__NR_open_tree, dfd, filename, flags);
+}
+
+/* Child exit codes. */
+enum {
+	CHILD_OK,		/* the operation failed with EINVAL as it must */
+	CHILD_ALLOWED,		/* the operation succeeded: the flag was lost */
+	CHILD_UNSHARE,		/* unshare(CLONE_NEWNS) failed */
+	CHILD_ERRNO,		/* the operation failed with some other errno */
+	CHILD_MOUNTINFO,	/* the mount was not found in mountinfo */
+};
+
+FIXTURE(mntns_unbindable)
+{
+	char base[64];
+	char src[80];
+	char dst[80];
+	bool mounted;
+};
+
+FIXTURE_SETUP(mntns_unbindable)
+{
+	self->mounted = false;
+
+	if (geteuid() != 0)
+		SKIP(return, "test requires CAP_SYS_ADMIN");
+
+	ASSERT_EQ(unshare(CLONE_NEWNS), 0);
+	ASSERT_EQ(mount("", "/", NULL, MS_REC | MS_PRIVATE, NULL), 0);
+
+	snprintf(self->base, sizeof(self->base), "/tmp/mntns_unbindable.XXXXXX");
+	ASSERT_NE(mkdtemp(self->base), NULL);
+	ASSERT_EQ(mount("tmpfs", self->base, "tmpfs", 0, NULL), 0);
+	self->mounted = true;
+
+	snprintf(self->src, sizeof(self->src), "%s/src", self->base);
+	snprintf(self->dst, sizeof(self->dst), "%s/dst", self->base);
+	ASSERT_EQ(mkdir(self->src, 0755), 0);
+	ASSERT_EQ(mkdir(self->dst, 0755), 0);
+
+	ASSERT_EQ(mount("tmpfs", self->src, "tmpfs", 0, NULL), 0);
+	ASSERT_EQ(mount(NULL, self->src, NULL, MS_UNBINDABLE, NULL), 0);
+}
+
+FIXTURE_TEARDOWN(mntns_unbindable)
+{
+	if (self->mounted)
+		umount2(self->base, MNT_DETACH);
+	rmdir(self->base);
+}
+
+static int classify(int ret, int err)
+{
+	if (ret >= 0)
+		return CHILD_ALLOWED;
+	return err == EINVAL ? CHILD_OK : CHILD_ERRNO;
+}
+
+/* Is the mount on @mountpoint marked unbindable in /proc/self/mountinfo? */
+static int mountinfo_unbindable(const char *mountpoint)
+{
+	char line[4096];
+	FILE *f;
+	int ret = CHILD_MOUNTINFO;
+
+	f = fopen("/proc/self/mountinfo", "re");
+	if (!f)
+		return CHILD_ERRNO;
+
+	while (fgets(line, sizeof(line), f)) {
+		char *fields[6], *p = line, *opt;
+		int i;
+
+		for (i = 0; i < 6; i++) {
+			fields[i] = strsep(&p, " ");
+			if (!fields[i])
+				break;
+		}
+		if (i < 6 || strcmp(fields[4], mountpoint))
+			continue;
+
+		/* the optional fields, up to the "-" separator */
+		ret = CHILD_ALLOWED;
+		while ((opt = strsep(&p, " ")) && strcmp(opt, "-")) {
+			if (!strcmp(opt, "unbindable"))
+				ret = CHILD_OK;
+		}
+		break;
+	}
+	fclose(f);
+	return ret;
+}
+
+static int run_in_child(int (*fn)(const char *src, const char *dst),
+			const char *src, const char *dst)
+{
+	int status;
+	pid_t pid;
+
+	pid = fork();
+	if (pid < 0)
+		return -1;
+	if (pid == 0)
+		_exit(fn(src, dst));
+	if (waitpid(pid, &status, 0) != pid || !WIFEXITED(status))
+		return -1;
+	return WEXITSTATUS(status);
+}
+
+static int bind_after_clone(const char *src, const char *dst)
+{
+	int ret;
+
+	if (unshare(CLONE_NEWNS))
+		return CHILD_UNSHARE;
+	ret = mount(src, dst, NULL, MS_BIND, NULL);
+	return classify(ret, errno);
+}
+
+static int rbind_after_clone(const char *src, const char *dst)
+{
+	int ret;
+
+	if (unshare(CLONE_NEWNS))
+		return CHILD_UNSHARE;
+	ret = mount(src, dst, NULL, MS_BIND | MS_REC, NULL);
+	return classify(ret, errno);
+}
+
+static int open_tree_after_clone(const char *src, const char *dst)
+{
+	int ret;
+
+	if (unshare(CLONE_NEWNS))
+		return CHILD_UNSHARE;
+	ret = sys_open_tree(AT_FDCWD, src, OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC);
+	return classify(ret, errno);
+}
+
+static int mountinfo_after_clone(const char *src, const char *dst)
+{
+	if (unshare(CLONE_NEWNS))
+		return CHILD_UNSHARE;
+	return mountinfo_unbindable(src);
+}
+
+static int bind_after_two_clones(const char *src, const char *dst)
+{
+	int ret;
+
+	if (unshare(CLONE_NEWNS))
+		return CHILD_UNSHARE;
+	if (unshare(CLONE_NEWNS))
+		return CHILD_UNSHARE;
+	ret = mount(src, dst, NULL, MS_BIND, NULL);
+	return classify(ret, errno);
+}
+
+/* The namespace the mount was made unbindable in. */
+TEST_F(mntns_unbindable, refuses_bind)
+{
+	int ret = mount(self->src, self->dst, NULL, MS_BIND, NULL);
+
+	ASSERT_EQ(classify(ret, errno), CHILD_OK);
+	ASSERT_EQ(mountinfo_unbindable(self->src), CHILD_OK);
+}
+
+/* A copy of the namespace must not turn the mount bindable. */
+TEST_F(mntns_unbindable, refuses_bind_after_clone)
+{
+	ASSERT_EQ(run_in_child(bind_after_clone, self->src, self->dst), CHILD_OK)
+		TH_LOG("bind of an unbindable mount allowed in a copied mount namespace");
+}
+
+TEST_F(mntns_unbindable, refuses_rbind_after_clone)
+{
+	ASSERT_EQ(run_in_child(rbind_after_clone, self->src, self->dst), CHILD_OK)
+		TH_LOG("rbind of an unbindable mount allowed in a copied mount namespace");
+}
+
+TEST_F(mntns_unbindable, refuses_open_tree_after_clone)
+{
+	ASSERT_EQ(run_in_child(open_tree_after_clone, self->src, self->dst), CHILD_OK)
+		TH_LOG("OPEN_TREE_CLONE of an unbindable mount allowed in a copied mount namespace");
+}
+
+TEST_F(mntns_unbindable, mountinfo_after_clone)
+{
+	ASSERT_EQ(run_in_child(mountinfo_after_clone, self->src, self->dst), CHILD_OK)
+		TH_LOG("mountinfo does not show the mount as unbindable in a copied mount namespace");
+}
+
+TEST_F(mntns_unbindable, refuses_bind_after_two_clones)
+{
+	ASSERT_EQ(run_in_child(bind_after_two_clones, self->src, self->dst), CHILD_OK)
+		TH_LOG("bind of an unbindable mount allowed two mount namespace copies down");
+}
+
+TEST_HARNESS_MAIN

tools/testing/selftests/filesystems/umount_propagation/Makefile

@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+TEST_GEN_PROGS := umount_propagation_test
+
+CFLAGS += -Wall -O2 -g $(KHDR_INCLUDES)
+
+include ../../lib.mk

tools/testing/selftests/filesystems/umount_propagation/umount_propagation_test.c

@@ -0,0 +1,226 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A synchronous umount fails with EBUSY when a mount it would pull out by
+ * propagation is still in use.
+ */
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <linux/mount.h>
+#include <linux/stat.h>
+
+#include "../../kselftest_harness.h"
+
+#ifndef OPEN_TREE_CLONE
+#define OPEN_TREE_CLONE		1
+#endif
+#ifndef OPEN_TREE_CLOEXEC
+#define OPEN_TREE_CLOEXEC	O_CLOEXEC
+#endif
+#ifndef AT_RECURSIVE
+#define AT_RECURSIVE		0x8000
+#endif
+#ifndef MOVE_MOUNT_F_EMPTY_PATH
+#define MOVE_MOUNT_F_EMPTY_PATH	0x00000004
+#endif
+#ifndef MOVE_MOUNT_BENEATH
+#define MOVE_MOUNT_BENEATH	0x00000200
+#endif
+#ifndef STATX_MNT_ID
+#define STATX_MNT_ID		0x00001000U
+#endif
+
+static int sys_open_tree(int dfd, const char *filename, unsigned int flags)
+{
+	return syscall(__NR_open_tree, dfd, filename, flags);
+}
+
+static int sys_move_mount(int from_dfd, const char *from_pathname,
+			  int to_dfd, const char *to_pathname,
+			  unsigned int flags)
+{
+	return syscall(__NR_move_mount, from_dfd, from_pathname, to_dfd,
+		       to_pathname, flags);
+}
+
+/* Child exit codes. */
+enum {
+	CHILD_OK,
+	CHILD_UNSHARE,		/* could not set up the slave namespace */
+	CHILD_OPEN_TREE,	/* open_tree() failed */
+	CHILD_MOVE_MOUNT,	/* move_mount() failed */
+	CHILD_STATX,		/* statx() failed */
+	CHILD_PIPE,		/* the parent went away */
+};
+
+/* Messages between parent and child. */
+enum {
+	MSG_READY = 'r',	/* child: the copy is mounted and referenced */
+	MSG_CHECK = 'c',	/* parent: check that the copy is still attached */
+	MSG_ATTACHED = 'a',	/* child: it is */
+	MSG_DETACHED = 'd',	/* child: it is not */
+	MSG_CLOSE = 'x',	/* parent: drop the reference */
+	MSG_CLOSED = 'y',	/* child: dropped */
+	MSG_EXIT = 'e',		/* parent: done */
+};
+
+FIXTURE(umount_propagation)
+{
+	char base[64];
+	char victim[80];
+	bool mounted;
+};
+
+FIXTURE_SETUP(umount_propagation)
+{
+	self->mounted = false;
+
+	if (geteuid() != 0)
+		SKIP(return, "test requires CAP_SYS_ADMIN");
+
+	ASSERT_EQ(unshare(CLONE_NEWNS), 0);
+	ASSERT_EQ(mount("", "/", NULL, MS_REC | MS_PRIVATE, NULL), 0);
+
+	snprintf(self->base, sizeof(self->base), "/tmp/umount_propagation.XXXXXX");
+	ASSERT_NE(mkdtemp(self->base), NULL);
+	ASSERT_EQ(mount("tmpfs", self->base, "tmpfs", 0, NULL), 0);
+	self->mounted = true;
+	ASSERT_EQ(mount(NULL, self->base, NULL, MS_SHARED, NULL), 0);
+
+	snprintf(self->victim, sizeof(self->victim), "%s/victim", self->base);
+	ASSERT_EQ(mkdir(self->victim, 0755), 0);
+	ASSERT_EQ(mount("tmpfs", self->victim, "tmpfs", 0, NULL), 0);
+}
+
+FIXTURE_TEARDOWN(umount_propagation)
+{
+	if (self->mounted)
+		umount2(self->base, MNT_DETACH);
+	rmdir(self->base);
+}
+
+static int send_msg(int fd, char msg)
+{
+	return write(fd, &msg, 1) == 1 ? 0 : -1;
+}
+
+static char recv_msg(int fd)
+{
+	char msg;
+
+	if (read(fd, &msg, 1) != 1)
+		return 0;
+	return msg;
+}
+
+/* Is the mount with id @mnt_id attached in this mount namespace? */
+static bool mount_attached(__u64 mnt_id)
+{
+	char line[4096];
+	bool found = false;
+	FILE *f;
+
+	f = fopen("/proc/self/mountinfo", "re");
+	if (!f)
+		return false;
+
+	while (fgets(line, sizeof(line), f)) {
+		if (strtoull(line, NULL, 10) == mnt_id) {
+			found = true;
+			break;
+		}
+	}
+	fclose(f);
+	return found;
+}
+
+/*
+ * The slave namespace: take a detached copy of the shared tree and move it
+ * beneath the propagated copy of the victim, keeping the open_tree()
+ * descriptor as a reference on it.
+ */
+static int slave_child(const char *base, const char *victim, int to_parent,
+		       int from_parent)
+{
+	struct statx stx;
+	int fd;
+
+	if (unshare(CLONE_NEWNS))
+		return CHILD_UNSHARE;
+	if (mount("", "/", NULL, MS_REC | MS_SLAVE, NULL))
+		return CHILD_UNSHARE;
+
+	fd = sys_open_tree(AT_FDCWD, base,
+			   OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC | AT_RECURSIVE);
+	if (fd < 0)
+		return CHILD_OPEN_TREE;
+	if (sys_move_mount(fd, "", AT_FDCWD, victim,
+			   MOVE_MOUNT_F_EMPTY_PATH | MOVE_MOUNT_BENEATH))
+		return CHILD_MOVE_MOUNT;
+	if (statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID, &stx))
+		return CHILD_STATX;
+
+	if (send_msg(to_parent, MSG_READY) || recv_msg(from_parent) != MSG_CHECK)
+		return CHILD_PIPE;
+	if (send_msg(to_parent, mount_attached(stx.stx_mnt_id) ?
+				MSG_ATTACHED : MSG_DETACHED))
+		return CHILD_PIPE;
+
+	if (recv_msg(from_parent) != MSG_CLOSE)
+		return CHILD_PIPE;
+	close(fd);
+	if (send_msg(to_parent, MSG_CLOSED) || recv_msg(from_parent) != MSG_EXIT)
+		return CHILD_PIPE;
+	return CHILD_OK;
+}
+
+TEST_F(umount_propagation, busy_copy_pulled_out)
+{
+	int to_child[2], to_parent[2];
+	int status;
+	pid_t pid;
+
+	ASSERT_EQ(pipe(to_child), 0);
+	ASSERT_EQ(pipe(to_parent), 0);
+
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0) {
+		close(to_child[1]);
+		close(to_parent[0]);
+		_exit(slave_child(self->base, self->victim, to_parent[1],
+				  to_child[0]));
+	}
+	close(to_child[0]);
+	close(to_parent[1]);
+
+	ASSERT_EQ(recv_msg(to_parent[0]), MSG_READY);
+
+	/* the copy in the slave namespace is in use */
+	ASSERT_EQ(umount2(self->victim, 0), -1);
+	ASSERT_EQ(errno, EBUSY);
+
+	ASSERT_EQ(send_msg(to_child[1], MSG_CHECK), 0);
+	ASSERT_EQ(recv_msg(to_parent[0]), MSG_ATTACHED);
+
+	/* and once it is not, the umount goes through */
+	ASSERT_EQ(send_msg(to_child[1], MSG_CLOSE), 0);
+	ASSERT_EQ(recv_msg(to_parent[0]), MSG_CLOSED);
+	ASSERT_EQ(umount2(self->victim, 0), 0);
+
+	ASSERT_EQ(send_msg(to_child[1], MSG_EXIT), 0);
+	ASSERT_EQ(waitpid(pid, &status, 0), pid);
+	ASSERT_TRUE(WIFEXITED(status));
+	ASSERT_EQ(WEXITSTATUS(status), CHILD_OK);
+}
+
+TEST_HARNESS_MAIN

tools/testing/selftests/move_mount_set_group/move_mount_set_group_test.c

@@ -146,17 +146,19 @@ static void null_endofword(char *word)
 	*word = '\0';
 }
 
-static bool is_shared_mount(const char *path)
+/* Does the mount on @path carry the optional field @field in mountinfo? */
+static bool mount_has_field(const char *path, const char *field)
 {
 	size_t len = 0;
 	char *line = NULL;
 	FILE *f = NULL;
+	bool found = false;
 
 	f = fopen("/proc/self/mountinfo", "re");
 	if (!f)
 		return false;
 
-	while (getline(&line, &len, f) != -1) {
+	while (!found && getline(&line, &len, f) != -1) {
 		char *opts, *target;
 
 		target = get_field(line, 4);
@@ -172,15 +174,29 @@ static bool is_shared_mount(const char *path)
 		if (strcmp(target, path) != 0)
 			continue;
 
-		null_endofword(opts);
-		if (strstr(opts, "shared:"))
-			return true;
+		/* the optional fields end at the "-" separator */
+		while (opts && *opts != '-') {
+			char *next = strchr(opts, ' ');
+
+			if (next)
+				*next++ = '\0';
+			if (!strncmp(opts, field, strlen(field))) {
+				found = true;
+				break;
+			}
+			opts = next;
+		}
 	}
 
 	free(line);
 	fclose(f);
 
-	return false;
+	return found;
+}
+
+static bool is_shared_mount(const char *path)
+{
+	return mount_has_field(path, "shared:");
 }
 
 /* Attempt to de-conflict with the selftests tree. */
@@ -372,4 +388,50 @@ TEST_F(move_mount_set_group, complex_sharing_copying)
 	ASSERT_EQ(is_shared_mount(SET_GROUP_A), 1);
 }
 
+#define SET_GROUP_B "/tmp/B"
+#define SET_GROUP_C "/tmp/C"
+
+/*
+ * An unbindable mount is neither shared nor a slave, so it must not be
+ * accepted as the target: with a slave source it would end up unbindable
+ * and a slave at the same time.
+ */
+TEST_F(move_mount_set_group, unbindable_target)
+{
+	bool ret;
+
+	ret = move_mount_set_group_supported();
+	ASSERT_GE(ret, 0);
+	if (!ret)
+		SKIP(return, "move_mount(MOVE_MOUNT_SET_GROUP) is not supported");
+
+	ASSERT_EQ(mount(NULL, SET_GROUP_A, NULL, MS_SHARED, 0), 0);
+
+	/* B: a slave of A's peer group */
+	ASSERT_EQ(mkdir(SET_GROUP_B, 0777), 0);
+	ASSERT_EQ(mount(SET_GROUP_A, SET_GROUP_B, NULL, MS_BIND, NULL), 0);
+	ASSERT_EQ(mount(NULL, SET_GROUP_B, NULL, MS_SLAVE, 0), 0);
+	ASSERT_TRUE(mount_has_field(SET_GROUP_B, "master:"));
+
+	/* C: unbindable */
+	ASSERT_EQ(mkdir(SET_GROUP_C, 0777), 0);
+	ASSERT_EQ(mount(SET_GROUP_A, SET_GROUP_C, NULL, MS_BIND, NULL), 0);
+	ASSERT_EQ(mount(NULL, SET_GROUP_C, NULL, MS_UNBINDABLE, 0), 0);
+	ASSERT_TRUE(mount_has_field(SET_GROUP_C, "unbindable"));
+
+	/* from a slave */
+	ASSERT_EQ(syscall(__NR_move_mount, AT_FDCWD, SET_GROUP_B,
+			  AT_FDCWD, SET_GROUP_C, MOVE_MOUNT_SET_GROUP), -1);
+	ASSERT_EQ(errno, EINVAL);
+	ASSERT_FALSE(mount_has_field(SET_GROUP_C, "master:"));
+	ASSERT_TRUE(mount_has_field(SET_GROUP_C, "unbindable"));
+
+	/* from a shared mount */
+	ASSERT_EQ(syscall(__NR_move_mount, AT_FDCWD, SET_GROUP_A,
+			  AT_FDCWD, SET_GROUP_C, MOVE_MOUNT_SET_GROUP), -1);
+	ASSERT_EQ(errno, EINVAL);
+	ASSERT_FALSE(mount_has_field(SET_GROUP_C, "shared:"));
+	ASSERT_TRUE(mount_has_field(SET_GROUP_C, "unbindable"));
+}
+
 TEST_HARNESS_MAIN