[PATCH 0/8] mount: a few gnarly fixes

[PATCH 0/8] mount: a few gnarly fixes

[Christian Brauner]

Hey,

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.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (8):
      mount: keep a copied mount unbindable
      selftests/filesystems: check that a copied mount namespace keeps unbindable
      mount: refuse MOVE_MOUNT_SET_GROUP on an unbindable mount
      selftests/move_mount_set_group: check that an unbindable target is refused
      fs: don't silently unmount busy mounts
      selftests/filesystems: check that a busy propagated copy blocks a synchronous umount
      fs: don't let a migrating task hide its reference from do_umount()
      docs: update the unmount propagation rule

 Documentation/filesystems/sharedsubtree.rst        |  20 +-
 fs/mount.h                                         |   3 +-
 fs/namespace.c                                     |  51 +++--
 fs/pnode.c                                         | 108 ++++++++--
 tools/testing/selftests/Makefile                   |   2 +
 .../filesystems/mntns_unbindable/Makefile          |   6 +
 .../mntns_unbindable/mntns_unbindable_test.c       | 227 +++++++++++++++++++++
 .../filesystems/umount_propagation/Makefile        |   6 +
 .../umount_propagation/umount_propagation_test.c   | 226 ++++++++++++++++++++
 .../move_mount_set_group_test.c                    |  74 ++++++-
 10 files changed, 673 insertions(+), 50 deletions(-)
---
base-commit: 2d2a2d7aa98741b58f54cacc99b52024e4d865f9
change-id: 20260923-work-mount-fixes-c9f19fb484eb

[PATCH 1/8] mount: keep a copied mount unbindable

[Christian Brauner]

It's groundhog day.

MNT_UNBINDABLE used to be set in in mnt->mnt.mnt_flags and was not part
of MNT_INTERNAL_FLAGS. clone_mnt() copied it with all the other flags to
the new mount. This guaranteed that the copy of an unbindable mount
became unbindable as well. Unbindable copies that ended up as shared
lost the unbindable property.

But then commit 406fea799925 ("mount: separate the flags accessed only
under namespace_sem") moved MNT_UNBINDABLE from mnt->mnt.mnt_flags into
mnt->mnt_t_flags as T_UNBINDABLE.

clone_mnt() doesn't copy from mnt_t_flags and specifically doesn't copy
T_UNBINDABLE. The only place a mount gets marked unbindable is in
change_mnt_propagation() for MS_UNBINDABLE.

This reintroduced an earlier bug we had already fixed. It resurfaces in
copy_mnt_ns() which clones unbindable mounts via CL_COPY_UNBINDABLE. So
since v6.17 a mount namespace created via clone(CLONE_NEWNS) or
unshare(CLONE_NEWNS) contain private and bindable copies of every
unbindable mount. The following snippet:

  mount --make-unbindable /mnt
  unshare -m --propagation unchanged
  mount --bind /mnt /tmp/x

succeeds. This used to fail with EINVAL. This also applies to
recursively binding a tree that contains an unbindable the mount. Such
subtrees used to be pruned when they were moved onto a shared mount.

And Documentation/filesystems/sharedsubtree.rst still says that the copy
of an unbindable mount is unbindable.

So once again let's fix this. Copy T_UNBINDABLE in clone_mnt(). Let
set_mnt_shared() mask it off whenever a mount is made shared.
This restores what the old MNT_INTERNAL_FLAGS mask did.

Fixes: 406fea799925 ("mount: separate the flags accessed only under namespace_sem")
Cc: stable@vger.kernel.org # v6.17+
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/namespace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index 580877e46b1a..a052f847c5df 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1254,6 +1254,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 */

-- 
2.53.0

[PATCH 5/8] fs: don't silently unmount busy mounts

[Christian Brauner]

The propagate_mount_busy() helper exists to decide whether a synchronous
can suceed. It does that by looking at the copies of the victims in the
mounts its parent propagates to. One of those functions from propagation
hell.

It skips a copy that has child mounts except for the case of a single
child mount covering the copy's root. That algorithm used to be exact.
It isn't anymore.

Originally only childless copies were unmounted during propagation.
And 1064f874abc0 ("mnt: Tuck mounts under others instead of creating
shadow/side mounts.") ended up adding covered mounts to both sides in
one go.

Along came 99b19d16471e ("mnt: In propgate_umount handle visiting mounts
in any order") and that started to unmount more than before:

[1]: A copy of a mount is unmounted when each of its children is either
     its overmount or a copy of the victim itself. In the former case
     the overmount gets reparented. In the latter case the copy of the
     victim will end up being unmounted.

So propagate_umount() ended up being rewritten without adjusting
propagate_mount_busy(). And currently propagate_mount_busy() doesn't
look at copies of the victim that have two or more children nor a single
child that is not the overmount.

So any such copy in [1] is not checked for
references. The result is that a synchronous unmount succeeds, pulling
out a mount that is still in use.

A container can run into this with its own mounts. The host shares a
tree with the container and the host has a mount in that tree:

  mount -t tmpfs none /mnt
  mount --make-shared /mnt
  mount -t tmpfs none /mnt/a

The container's /mnt is a slave copy of the host mount. The container
now takes a detached copy of the tree and mounts that copy beneath its
propagated copy of /mnt/a and keeps the file descriptor to the tree
open:

  unshare -m --propagation unchanged
  mount --make-rslave /
  fd = open_tree(AT_FDCWD, "/mnt", OPEN_TREE_CLONE | AT_RECURSIVE)
  move_mount(fd, "", AT_FDCWD, "/mnt/a", MOVE_MOUNT_BENEATH)

  grep /mnt /proc/self/mountinfo
  775 355 ... /mnt ...     master:646
  777 775 ... /mnt/a ...   master:646
  776 777 ... /mnt/a ...   master:647
  778 777 ... /mnt/a/a ... master:647

The now attached tree (777) is now mounted beneath the container's copy
of /mnt/a. That's also where the host mount sits (as seen from the host
mount namespace). So the propagated copy (776) of the host's mount is on
top of its root and the copy of that (778) is inside it. Now the host
runs:

  umount /mnt/a

and it succeeds. The moved tree tree and the copy inside of it are gone
from the container. Now only the propagated copy is left and it got
reparented to where it was before:

  grep /mnt /proc/self/mountinfo
  775 355 ... /mnt ... master:646
  776 775 ... /mnt/a ...

But the file descriptor still refers to the moved tree. So a synchronous
umount detached a mount that is still in use.

Teach propagate_mount_busy() the same checks that propagate_umount()
does. It now checks for a single victim mount without children
for which it can assume that MNT_LOCKED is cleared on every copy like
propagate_umount() does. The copies form chains. IOW, when a copy itself
receives propagation from the victim's parent then the mount at the same
mountpoint inside that copy is a copy as well. Whether a mount receives
propagation from the victim's can simply be checked by walking its
masters. Walk each chain once from its top so we are sure that nested
copies stay linear. Check every reference once of every copy that gets
unmounted.

Fixes: 99b19d16471e ("mnt: In propgate_umount handle visiting mounts in any order")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/pnode.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 90 insertions(+), 18 deletions(-)

diff --git a/fs/pnode.c b/fs/pnode.c
index 5d91c3e58d2a..2cd667958efe 100644
--- a/fs/pnode.c
+++ b/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;

-- 
2.53.0

Re: [PATCH 5/8] fs: don't silently unmount busy mounts

[Al Viro]

[My apologies for being MIA since July - had been sick; should be
back to normal now]

On Wed, Sep 23, 2026 at 02:27:57PM +0200, Christian Brauner wrote:
> The propagate_mount_busy() helper exists to decide whether a synchronous
> can suceed. It does that by looking at the copies of the victims in the
> mounts its parent propagates to. One of those functions from propagation
> hell.
> 
> It skips a copy that has child mounts except for the case of a single
> child mount covering the copy's root. That algorithm used to be exact.
> It isn't anymore.
> 
> Originally only childless copies were unmounted during propagation.
> And 1064f874abc0 ("mnt: Tuck mounts under others instead of creating
> shadow/side mounts.") ended up adding covered mounts to both sides in
> one go.
> 
> Along came 99b19d16471e ("mnt: In propgate_umount handle visiting mounts
> in any order") and that started to unmount more than before:
> 
> [1]: A copy of a mount is unmounted when each of its children is either
>      its overmount or a copy of the victim itself. In the former case
>      the overmount gets reparented. In the latter case the copy of the
>      victim will end up being unmounted.
> 
> So propagate_umount() ended up being rewritten without adjusting
> propagate_mount_busy(). And currently propagate_mount_busy() doesn't
> look at copies of the victim that have two or more children nor a single
> child that is not the overmount.
> 
> So any such copy in [1] is not checked for
> references. The result is that a synchronous unmount succeeds, pulling
> out a mount that is still in use.
> 
> A container can run into this with its own mounts. The host shares a
> tree with the container and the host has a mount in that tree:
> 
>   mount -t tmpfs none /mnt
>   mount --make-shared /mnt
nit: mkdir /mnt/a
>   mount -t tmpfs none /mnt/a
> 
> The container's /mnt is a slave copy of the host mount. The container
> now takes a detached copy of the tree and mounts that copy beneath its
> propagated copy of /mnt/a and keeps the file descriptor to the tree
> open:
> 
>   unshare -m --propagation unchanged
>   mount --make-rslave /
>   fd = open_tree(AT_FDCWD, "/mnt", OPEN_TREE_CLONE | AT_RECURSIVE)
>   move_mount(fd, "", AT_FDCWD, "/mnt/a", MOVE_MOUNT_BENEATH)
> 
>   grep /mnt /proc/self/mountinfo
>   775 355 ... /mnt ...     master:646
>   777 775 ... /mnt/a ...   master:646
>   776 777 ... /mnt/a ...   master:647
>   778 777 ... /mnt/a/a ... master:647
> 
> The now attached tree (777) is now mounted beneath the container's copy
> of /mnt/a. That's also where the host mount sits (as seen from the host
> mount namespace). So the propagated copy (776) of the host's mount is on
> top of its root and the copy of that (778) is inside it. Now the host
> runs:
> 
>   umount /mnt/a
> 
> and it succeeds. The moved tree tree and the copy inside of it are gone
> from the container. Now only the propagated copy is left and it got
> reparented to where it was before:
> 
>   grep /mnt /proc/self/mountinfo
>   775 355 ... /mnt ... master:646
>   776 775 ... /mnt/a ...
> 
> But the file descriptor still refers to the moved tree. So a synchronous
> umount detached a mount that is still in use.
> 
> Teach propagate_mount_busy() the same checks that propagate_umount()
> does. It now checks for a single victim mount without children
> for which it can assume that MNT_LOCKED is cleared on every copy like
> propagate_umount() does.

Huh?  propagate_umount() does *not* assume anything of that sort - look for
those IS_MNT_LOCKED() in there.

> The copies form chains. IOW, when a copy itself
> receives propagation from the victim's parent then the mount at the same
> mountpoint inside that copy is a copy as well. Whether a mount receives
> propagation from the victim's can simply be checked by walking its
> masters. Walk each chain once from its top so we are sure that nested
> copies stay linear. Check every reference once of every copy that gets
> unmounted.

Explanation is somewhat misleading here, IMO...  Sure, all candidate
victims have identical mountpoints, so they must have distinct parents,
etc., but "copy" is not a good word here - all of them *are* copies of
copies of the same mount, but the history of copying might have nothing
to do with your chains.

	FWIW, there are two related bugs here; one is the horror with
MOVE_MOUNT_BENEATH (and the more I'm thinking of interplay between that
and propagation, the less I like the entire situation).  Another does
not involve MOVE_MOUNT_BENEATH at all - 

; mount --type tmpfs none /mnt    # M0
; mount --make-private /mnt
; mount --make-shared /mnt
; mkdir /mnt/a
; mkdir /mnt/b
; mount --bind /mnt /mnt/a        # M1 = M0/a
; mount --make-shared /mnt/a
; mount --bind /mnt/a /mnt/b      # M2 = M0/b
; mount --make-slave /mnt/a
; mount --make-slave /mnt/b
; umount /mnt/a/b
; mount --bind /mnt/b /mnt/b/a    # M3 = M2/a
; mount --bind /mnt/b /mnt/b/a/a  # M4 = M3/a
; cd /mnt/b/a			# M3 is busy, isn't it?
; /bin/pwd
/mnt/b/a
; umount /mnt/a			# takes out M1, M3 and M4
; /bin/pwd
pwd: couldn't find directory entry in '..' with matching i-node

What happens is that propagate_mount_busy() sees that M3 has something
mounted _not_ on its root and decides that propagate_umount() will not
take it out.  M1 and M4 are not busy, so we proceed to propagate_umount(),
which notices what propagate_mount_busy() hadn't - the thing mounted on
M3/a is something we are taking out, so M3 is not pinned by remaining
mounts...

I'm not fond of the patch itself, TBH - the amount of work done under
mount_lock would better be linear by the number of mounts involved and
your variant is at least quadratic.  That's not hard to fix, but that'll
obviously have to wait - Saturday after -rc7 is not a good time for
that.

Re: [PATCH 5/8] fs: don't silently unmount busy mounts

[Christian Brauner]

On Sun, Oct 18, 2026 at 02:38:15AM +0100, Al Viro wrote:
> [My apologies for being MIA since July - had been sick; should be
> back to normal now]

Glad you're better!

> >   mount --make-shared /mnt
> nit: mkdir /mnt/a
> >   mount -t tmpfs none /mnt/a

Yep, lost that line.

> > Teach propagate_mount_busy() the same checks that propagate_umount()
> > does. It now checks for a single victim mount without children
> > for which it can assume that MNT_LOCKED is cleared on every copy like
> > propagate_umount() does.
> 
> Huh?  propagate_umount() does *not* assume anything of that sort - look for
> those IS_MNT_LOCKED() in there.

Badly worded, sorry. What I meant is that in the do_umount() path
propagate_mount_unlock() has cleared MNT_LOCKED on every candidate by
the time propagate_umount() looks at them. It's the same set. The mount
at the mountpoint below each receiver. So for a single childless victim
none of the IS_MNT_LOCKED() checks can trigger and
propagate_mount_busy() doesn't need to mirror them.

> Explanation is somewhat misleading here, IMO...  Sure, all candidate
> victims have identical mountpoints, so they must have distinct parents,
> etc., but "copy" is not a good word here - all of them *are* copies of
> copies of the same mount, but the history of copying might have nothing
> to do with your chains.

Fair. "copy" was meant as "receives propagation from the victim's
parent", the clone history doesn't matter.

> ; umount /mnt/a			# takes out M1, M3 and M4
> ; /bin/pwd
> pwd: couldn't find directory entry in '..' with matching i-node
> 
> What happens is that propagate_mount_busy() sees that M3 has something
> mounted _not_ on its root and decides that propagate_umount() will not
> take it out.  M1 and M4 are not busy, so we proceed to propagate_umount(),
> which notices what propagate_mount_busy() hadn't - the thing mounted on
> M3/a is something we are taking out, so M3 is not pinned by remaining
> mounts...

That one is covered. M3 is the first candidate in the chain below M2
and M4 is its next candidate, so chain_busy() does the refcount check
on M3 and umount(2) fails with EBUSY. I ran your sequence on current
mainline: EBUSY, cwd intact. On v7.1 it takes M3 out as you describe.

> I'm not fond of the patch itself, TBH - the amount of work done under
> mount_lock would better be linear by the number of mounts involved and
> your variant is at least quadratic.  That's not hard to fix, but that'll
> obviously have to wait - Saturday after -rc7 is not a good time for
> that.

Agreed, it isn't pretty. receives_from() walks the master chain for
every candidate and chain_busy() walks every chain twice. But I very
much didn't want to touch propagate_umount() in something that goes to
stable.

For 7.4 the obvious way is to run gather_candidates() and the trim
passes without committing anything and refcount check whatever ends up
in to_umount. If you have something send it my way, otherwise I'll do it
for 7.4.