[PATCH 00/10] mount: a few additional bugfixes

[PATCH 00/10] mount: a few additional bugfixes

[Christian Brauner]

Here's a set of additional mount fixes that fell out of my recent look
at the mount code:

- Fix a UAF in mnt_change_mountpoint().
- Fix a UAF in statmount().
- FIX a UAF and various associated bugs in clone_mnt()
- Prevent copying an unmounted mount tree.
- Avoid pointless work in mount_setattr().
- Refuse fspick() on kernel internal superblocks.
- Simplify dissolve_on_fput() and unshare().

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (10):
      namespace: reset the old parent's ->overmount in mnt_change_mountpoint()
      statmount: read the parent of an unmounted mount under mount_lock
      namespace: don't inherit MNT_UMOUNT in clone_mnt()
      namespace: don't copy an unmounted mount tree
      namespace: check the target before walking it in do_mount_setattr()
      fs: refuse fspick() on internal superblocks
      fs: put the old fs_struct before the old namespaces in unshare()
      namespace: drop the file's reference first in dissolve_on_fput()
      selftests/filesystems: check that an unmounted mount tree isn't walked
      selftests/filesystems: check that a dead mount's overmount isn't followed

 fs/file_table.c                                    |   3 +-
 fs/fsopen.c                                        |   3 +
 fs/namespace.c                                     |  37 +-
 include/linux/mount.h                              |   2 +-
 kernel/fork.c                                      |   9 +-
 tools/testing/selftests/Makefile                   |   1 +
 .../selftests/filesystems/mount_cycle/.gitignore   |   3 +
 .../selftests/filesystems/mount_cycle/Makefile     |   6 +
 .../mount_cycle/overmount_reparent_test.c          | 181 ++++++++
 .../filesystems/mount_cycle/unmounted_tree_test.c  | 484 +++++++++++++++++++++
 10 files changed, 719 insertions(+), 10 deletions(-)
---
base-commit: 762a6411a5a7aa4119570bae590662312ccb4be3
change-id: 20260926-work-mount-fixes-2-90d619ace163

[PATCH 01/10] namespace: reset the old parent's ->overmount in mnt_change_mountpoint()

[Christian Brauner]

__umount_mnt() resets ->overmount of the parent when it unmounts the
mount that overmounts it. mnt_change_mountpoint() doesn't. It moves the
mount to its new parent and leaves the old one pointing at it.

For a mount that is moved beneath in attach_recursive_mnt() that doesn't
matter. The new mount has already reset ->overmount of the old parent by
the time the old top mount is moved onto it.

It matters for reparent() though. propagate_umount() moves a surviving
overmount onto the first ancestor that isn't going to be unmounted. The
mount it was taken from is unmounted immediately afterwards. If a file
descriptor keeps that unmounted mount alive ->overmount points at the
moved mount. That's a dangling pointer and UAF as the moved mount is
unmounted and freed.

do_lock_mount() follows ->overmount from the target path under
mount_lock. With a file descriptor on such a mount as the target of a
mount beneath topmost_overmount() walks into the freed mount and
do_lock_mount() takes references on whatever it finds there.

So reset ->overmount of the old parent in mnt_change_mountpoint() the
same way __umount_mnt() does. A mount's ->overmount is then always a
mount that is attached to it.

Fixes: ffdc52fbbd58 ("prevent mount hash conflicts")
Cc: stable@vger.kernel.org # v6.17+
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/namespace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index 73215f84e04d..230f95675a93 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1069,7 +1069,10 @@ static void attach_mnt(struct mount *mnt, struct mount *parent,
 void mnt_change_mountpoint(struct mount *parent, struct mountpoint *mp, struct mount *mnt)
 {
 	struct mountpoint *old_mp = mnt->mnt_mp;
+	struct mount *old_parent = mnt->mnt_parent;
 
+	if (old_parent->overmount == mnt)
+		old_parent->overmount = NULL;
 	list_del_init(&mnt->mnt_child);
 	hlist_del_init(&mnt->mnt_mp_list);
 	hlist_del_init_rcu(&mnt->mnt_hash);

-- 
2.53.0

[PATCH 02/10] statmount: read the parent of an unmounted mount under mount_lock

[Christian Brauner]

statmount_mnt_basic() reads the ids of the parent through m->mnt_parent
with namespace_sem held for reading. That's fine for a mount that is
in a namespace. Everything above it is pinned by that namespace.

STATMOUNT_BY_FD also accepts a file descriptor on a mount that has
been unmounted on purpose and that only the file descriptor keeps alive.
If that mount stayed attached to its parent when it was unmounted the
parent is unmounted as well and nothing pins it for the child.

The final mntput() unmounts the children under mount_lock alone and
frees the parent afterwards. Concurrently, statmount_mnt_basic() reads
m->mnt_parent twice while that can happen and can easily dereference a
mount that's already gone.

Read the parent of an unmounted mount under mount_lock. The parent
can't be put while it is held and both ids come from the same mount. A
mounted mount's parent is pinned by the namespace and a mount can't
leave its namespace while namespace_sem is held, so the common case
takes no global lock.

Fixes: 0e5032237ee5 ("statmount: accept fd as a parameter")
Cc: stable@vger.kernel.org # v7.0+
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/namespace.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 230f95675a93..d1e8dd2e1e8e 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -5335,15 +5335,25 @@ static void statmount_sb_basic(struct kstatmount *s)
 	s->sm.sb_flags = sb->s_flags & (SB_RDONLY|SB_SYNCHRONOUS|SB_DIRSYNC|SB_LAZYTIME);
 }
 
+static void statmount_mnt_parent(struct kstatmount *s, const struct mount *m)
+{
+	s->sm.mnt_parent_id = m->mnt_parent->mnt_id_unique;
+	s->sm.mnt_parent_id_old = m->mnt_parent->mnt_id;
+}
+
 static void statmount_mnt_basic(struct kstatmount *s)
 {
 	struct mount *m = real_mount(s->mnt);
 
 	s->sm.mask |= STATMOUNT_MNT_BASIC;
 	s->sm.mnt_id = m->mnt_id_unique;
-	s->sm.mnt_parent_id = m->mnt_parent->mnt_id_unique;
 	s->sm.mnt_id_old = m->mnt_id;
-	s->sm.mnt_parent_id_old = m->mnt_parent->mnt_id;
+	/* An unmounted mount is cut loose from its parent under mount_lock alone. */
+	if (likely(is_mounted(s->mnt)))
+		statmount_mnt_parent(s, m);
+	else
+		scoped_guard(mount_locked_reader)
+			statmount_mnt_parent(s, m);
 	s->sm.mnt_attr = mnt_to_attr_flags(&m->mnt);
 	s->sm.mnt_propagation = mnt_to_propagation_flags(m);
 	s->sm.mnt_peer_group = m->mnt_group_id;

-- 
2.53.0

[PATCH 03/10] namespace: don't inherit MNT_UMOUNT in clone_mnt()

[Christian Brauner]

MNT_UMOUNT gets raised on a mount that umount_tree() or umount_one() has
taken out of its namespace. It's never cleared. __detach_mounts()
unhooks a mount that carries it instead of unmounting it.

disconnect_mount() lets an unmounted child stay attached only if its
parent carries it. propagate_umount() takes it as the sign that a mount
is going to be unmounted when it decides what else can be unmounted,
where a chain of locked mounts ends and where a for reparenting an
overmount.

clone_mnt() copies the flags of the source and only masks
MNT_INTERNAL_FLAGS which doesn't include MNT_UMOUNT. A mount that was
lazily unmounted but is kept alive by a file descriptor can still be
copied when its root is an nsfs or pidfs file because may_copy_tree()
accepts those before checking whether the source is mounted at all:

  mount --bind /proc/self/ns/net /tmp/x
  exec 3< /tmp/x
  umount -l /tmp/x
  mount --bind /proc/self/fd/3 /tmp/y

The mount on /tmp/y is attached to the caller's namespace with
MNT_UMOUNT already set and every copy of it inherits the flag again.

That causes a number of bugs:

- Unlink its mountpoint from a mount namespace where it isn't one.
  __detach_mounts() takes it for a mount that was unmounted earlier,
  unhooks it from its parent and drops the reference the namespace
  holds on it. It stays in the namespace with one reference too few
  and as its own parent, so it can't be unmounted through the file
  descriptor anymore and the namespace never finds it again. When the
  namespace goes away it's leaked and points to a freed struct
  mnt_namespace that may_change_propagation() and anon_ns_root() read
  on the next propagation change or mount_setattr() through the file
  descriptor.

- Make it shared and unmount a peer. trace_transfers() follows the
  flag from the peer into it and strips it from its peer group.

- Let a propagated umount reach it. gather_candidates() marks it with
  T_UMOUNT_CANDIDATE but never puts it on the candidate list, so the
  mark is never cleared and every later propagated umount skips it. If
  it's the parent of a locked mount handle_locked() takes it as the
  cutoff and commits the locked child that should have stayed and
  disconnect_mount() leaves that child attached to it, unmounted under
  a mounted parent, which is what disconnect_mount() exists to
  prevent. When the namespace goes away umount_tree() visits the child
  a second time. And once __detach_mounts() has made it its own parent
  reparent() never finds the end of the chain of unmounted parents.

A clone is a new mount that has never been unmounted. Add MNT_UMOUNT
to MNT_INTERNAL_FLAGS so clone_mnt() never copies it. And warn when
umount_tree() reaches a mount that already has it set.

Fixes: 590ce4bcbfb4 ("mnt: Add MNT_UMOUNT flag")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/namespace.c        | 2 ++
 include/linux/mount.h | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index d1e8dd2e1e8e..6d4ecde682df 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1799,6 +1799,8 @@ static void umount_tree(struct mount *mnt, enum umount_tree_flags how)
 
 	/* Gather the mounts to umount */
 	for (p = mnt; p; p = next_mnt(p, mnt)) {
+		/* A mount is unmounted once. */
+		VFS_WARN_ON_ONCE(p->mnt.mnt_flags & MNT_UMOUNT);
 		p->mnt.mnt_flags |= MNT_UMOUNT;
 		if (mnt_ns_attached(p))
 			move_from_ns(p);
diff --git a/include/linux/mount.h b/include/linux/mount.h
index acfe7ef86a1b..b16ba5d45605 100644
--- a/include/linux/mount.h
+++ b/include/linux/mount.h
@@ -52,7 +52,7 @@ enum mount_flags {
 	MNT_ATIME_MASK = MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME,
 
 	MNT_INTERNAL_FLAGS = MNT_INTERNAL | MNT_DOOMED |
-			     MNT_SYNC_UMOUNT | MNT_LOCKED
+			     MNT_SYNC_UMOUNT | MNT_LOCKED | MNT_UMOUNT
 };
 
 struct vfsmount {

-- 
2.53.0

[PATCH 04/10] namespace: don't copy an unmounted mount tree

[Christian Brauner]

may_copy_tree() accepts a mount whose root is an nsfs or pidfs file
before it checks whether that mount is mounted. This exception exists
for nsfs and pidfs mounts. They are part of no mount namespace so
neither check_mnt() nor is_mounted() would let them through and
bind-mounting a nsfsfd or a pidfd wouldn't work.

Problem is that exception also admits a bind mount of such a file when
it was lazily unmounted and is only kept alive by a file descriptor:

  mount --bind /proc/self/ns/net /tmp/x
  exec 3< /tmp/x
  umount -l /tmp/x
  mount --rbind /proc/self/fd/3 /tmp/y

With MS_REC or AT_RECURSIVE copy_tree() doesn't stop at the unmounted
mount. It copies every mount that stayed attached to it as well. So if
you have a bunch of overmounts over the nsfsfd or the pidfd the whole
unmounted stack is copied. That's a bug.

So refuse to walk an unmounted tree. Copying its root alone stays
possible. So a bind mount of a namespace that is gone can still be
recreated from a file descriptor on the old one. The file descriptor
pins that mount and its root and __has_locked_children() only reads the
direct children which don't leave the parent's list without
namespace_sem. The internal mounts carry MNT_NS_INTERNAL instead of NULL
and are still walked. Copying an unmounted tree recursively fails with
EINVAL.

Fixes: e149ed2b805f ("take the targets of /proc/*/ns/* symlinks to separate fs")
Fixes: ef4144ac2dec ("pidfs: allow bind-mounts")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/namespace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index 6d4ecde682df..c7e252eb249e 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3007,6 +3007,9 @@ static struct mount *__do_loopback(const struct path *old_path,
 	if (!may_copy_tree(old_path))
 		return ERR_PTR(-EINVAL);
 
+	if (recurse && !old->mnt_ns)
+		return ERR_PTR(-EINVAL);
+
 	if (!recurse && __has_locked_children(old, old_path->dentry))
 		return ERR_PTR(-EINVAL);
 

-- 
2.53.0

[PATCH 05/10] namespace: check the target before walking it in do_mount_setattr()

[Christian Brauner]

When mount_setattr() changes the propagation type to MS_SHARED
do_mount_setattr() takes namespace_sem and calls invent_group_ids()
which walks the whole target tree with next_mnt() and allocates a peer
group id for every mount that doesn't have one.

Whether the caller may touch the target at all is only checked
afterwards under mount_lock. If it may not, cleanup_group_ids() walks
the tree a second time on the way out to free the ids again.

A detached tree that a file descriptor keeps alive, a mount in another
mount namespace or a mount below the root of an anonymous mount
namespace does a bunch of allocation work that is completely useless
because it will fail and revert right after.

Do the right thing and check the target right after taking namespace_sem
instead. A mount leaves its namespace only under namespace_sem so the
mount stays stable. The check under mount_lock stays for the case where
no propagation change is requested and namespace_sem isn't taken.
Callers get the same EINVAL as before.

Fixes: 2a1867219c7b ("fs: add mount_setattr()")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/namespace.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index c7e252eb249e..de28fe9058ab 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4976,6 +4976,11 @@ static int do_mount_setattr(const struct path *path, struct mount_kattr *kattr)
 		 * propagation.
 		 */
 		namespace_lock();
+		/* invent_group_ids() walks the tree, only walk a mounted one */
+		if (!anon_ns_root(mnt) && !check_mnt(mnt)) {
+			namespace_unlock();
+			return -EINVAL;
+		}
 		if (kattr->propagation == MS_SHARED) {
 			err = invent_group_ids(mnt, kattr->kflags & MOUNT_KATTR_RECURSE);
 			if (err) {
@@ -4988,6 +4993,7 @@ static int do_mount_setattr(const struct path *path, struct mount_kattr *kattr)
 	err = -EINVAL;
 	lock_mount_hash();
 
+	/* Checked under namespace_sem already if the propagation changes. */
 	if (!anon_ns_root(mnt) && !check_mnt(mnt))
 		goto out;
 

-- 
2.53.0

[PATCH 06/10] fs: refuse fspick() on internal superblocks

[Christian Brauner]

There's a few instances where a internal superblock is reachable by
fspick(). Block that. graft_tree() and fsmount() already refuse them.
fanotify refuses mount and superblock marks on them for the same reason.

Fixes: cf3cba4a429b ("vfs: syscall: Add fspick() to select a superblock for reconfiguration")
Cc: stable@vger.kernel.org # v5.2+
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/fsopen.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/fsopen.c b/fs/fsopen.c
index ae19e5136598..9d5a7a22b529 100644
--- a/fs/fsopen.c
+++ b/fs/fsopen.c
@@ -190,6 +190,9 @@ SYSCALL_DEFINE3(fspick, int, dfd, const char __user *, path, unsigned int, flags
 	ret = -EINVAL;
 	if (target.mnt->mnt_root != target.dentry)
 		goto err_path;
+	/* kernel-internal superblocks are nobody's to reconfigure */
+	if (target.dentry->d_sb->s_flags & SB_NOUSER)
+		goto err_path;
 
 	fc = fs_context_for_reconfigure(target.dentry, 0, 0);
 	if (IS_ERR(fc)) {

-- 
2.53.0

Re: [PATCH 03/10] namespace: don't inherit MNT_UMOUNT in clone_mnt()

[Al Viro]

On Sat, Sep 26, 2026 at 03:44:26PM +0200, Christian Brauner wrote:

> clone_mnt() copies the flags of the source and only masks
> MNT_INTERNAL_FLAGS which doesn't include MNT_UMOUNT. A mount that was
> lazily unmounted but is kept alive by a file descriptor can still be
> copied when its root is an nsfs or pidfs file because may_copy_tree()
> accepts those before checking whether the source is mounted at all:
> 
>   mount --bind /proc/self/ns/net /tmp/x
>   exec 3< /tmp/x
>   umount -l /tmp/x
>   mount --bind /proc/self/fd/3 /tmp/y
> 
> The mount on /tmp/y is attached to the caller's namespace with
> MNT_UMOUNT already set and every copy of it inherits the flag again.
> 
> That causes a number of bugs:

[obviously]

I agree about adding it to MNT_INTERNAL_FLAGS, but I wonder if we should
ever allow to clone MNT_UMOUNT ones in the first place...

Re: [PATCH 03/10] namespace: don't inherit MNT_UMOUNT in clone_mnt()

[Christian Brauner]

On Sun, Oct 18, 2026 at 05:32:45AM +0100, Al Viro wrote:
> I agree about adding it to MNT_INTERNAL_FLAGS, but I wonder if we should
> ever allow to clone MNT_UMOUNT ones in the first place...

The next patch refuses to copy an unmounted tree recursively. Copying
just the root is kept on purpose...

Becaues a bind mount of an nsfs or pidfs file that got lazily unmounted
but is still pinned by an fd can be recreated from that fd and I'd
rather not break that...

With the flag not inherited anymore the copy is just a regular mount
with a reference on the old root. So I don't think refusing it buys us
anything.