* [PATCH 1/8] mount: keep a copied mount unbindable
2026-09-23 12:27 [PATCH 0/8] mount: a few gnarly fixes Christian Brauner
@ 2026-09-23 12:27 ` Christian Brauner
2026-09-23 12:27 ` [PATCH 2/8] selftests/filesystems: check that a copied mount namespace keeps unbindable Christian Brauner
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2026-09-23 12:27 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Alexander Viro, Jan Kara,
Christian Brauner (Amutable), stable
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/8] selftests/filesystems: check that a copied mount namespace keeps unbindable
2026-09-23 12:27 [PATCH 0/8] mount: a few gnarly fixes Christian Brauner
2026-09-23 12:27 ` [PATCH 1/8] mount: keep a copied mount unbindable Christian Brauner
@ 2026-09-23 12:27 ` Christian Brauner
2026-09-23 12:27 ` [PATCH 3/8] mount: refuse MOVE_MOUNT_SET_GROUP on an unbindable mount Christian Brauner
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2026-09-23 12:27 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Alexander Viro, Jan Kara,
Christian Brauner (Amutable)
Make a tmpfs unbindable, copy the mount namespace and check from the
copy that:
- a bind of the mount fails with EINVAL
- a recursive bind fails as well
- open_tree(OPEN_TREE_CLONE) of it fails
- mountinfo still shows it as unbindable
- a copy of the copy refuses the bind too
The first case runs in the original namespace so the fixture stays
honest about what it set up.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
tools/testing/selftests/Makefile | 1 +
.../filesystems/mntns_unbindable/Makefile | 6 +
.../mntns_unbindable/mntns_unbindable_test.c | 227 +++++++++++++++++++++
3 files changed, 234 insertions(+)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 273853937c25..a3df9a15ebb7 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -50,6 +50,7 @@ TARGETS += filesystems/empty_mntns
TARGETS += filesystems/fsmount_ns
TARGETS += filesystems/fscontext_ns
TARGETS += filesystems/xattr
+TARGETS += filesystems/mntns_unbindable
TARGETS += firmware
TARGETS += fpu
TARGETS += ftrace
diff --git a/tools/testing/selftests/filesystems/mntns_unbindable/Makefile b/tools/testing/selftests/filesystems/mntns_unbindable/Makefile
new file mode 100644
index 000000000000..33a311c5bd72
--- /dev/null
+++ b/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
diff --git a/tools/testing/selftests/filesystems/mntns_unbindable/mntns_unbindable_test.c b/tools/testing/selftests/filesystems/mntns_unbindable/mntns_unbindable_test.c
new file mode 100644
index 000000000000..9aebc37cf74d
--- /dev/null
+++ b/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
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/8] mount: refuse MOVE_MOUNT_SET_GROUP on an unbindable mount
2026-09-23 12:27 [PATCH 0/8] mount: a few gnarly fixes Christian Brauner
2026-09-23 12:27 ` [PATCH 1/8] mount: keep a copied mount unbindable Christian Brauner
2026-09-23 12:27 ` [PATCH 2/8] selftests/filesystems: check that a copied mount namespace keeps unbindable Christian Brauner
@ 2026-09-23 12:27 ` Christian Brauner
2026-09-23 12:27 ` [PATCH 4/8] selftests/move_mount_set_group: check that an unbindable target is refused Christian Brauner
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2026-09-23 12:27 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Alexander Viro, Jan Kara,
Christian Brauner (Amutable)
do_set_group() only accepts mounts as targets that are neither shared
nor a slave. That encompasses undindable mounts.
If the source mount is a slave mount the target mount will end up with
source's master as its master. It keeps T_UNBINDABLE. That means we get
a mount that is both unbindable and a slave:
mount --bind /tmp/src /tmp/src
mount --make-shared /tmp/src
mount --bind /tmp/src /tmp/b
mount --make-slave /tmp/b
mount --bind /tmp/src /tmp/c
mount --make-unbindable /tmp/c
move_mount(b, "", c, "", MOVE_MOUNT_SET_GROUP)
grep /tmp/c /proc/self/mountinfo
... /tmp/c ... master:646 unbindable ...
This property cannot be produced any other way. change_mnt_propagation()
drops the master when it makes a mount unbindable (iow, it becomes
private) and doesn't touch an unbindable mount when it is supposed to
turned into a slave mount.
End-result is that the unbindable-slave mount receives everything
propagated from its master's peer group while it can't be bind mounted
itself. Not sure what that's supposed to do.
On the other side: if the source mount is shared, the target mount drops
T_UNBINDABLE because because set_mnt_shared() clears T_SHARED_MASK.
So teach do_set_group() to refuse an unbindable target mount the same
way a shared or slave target mount is refused.
The main user of MOVE_MOUNT_SET_GROUP is CRIU which restores sharing
first and applies MS_UNBINDABLE afterwards. It never hits this.
Fixes: 9ffb14ef61ba ("move_mount: allow to add a mount into an existing group")
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/namespace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index a052f847c5df..d842fc1f1f1a 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3468,7 +3468,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 */
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/8] selftests/move_mount_set_group: check that an unbindable target is refused
2026-09-23 12:27 [PATCH 0/8] mount: a few gnarly fixes Christian Brauner
` (2 preceding siblings ...)
2026-09-23 12:27 ` [PATCH 3/8] mount: refuse MOVE_MOUNT_SET_GROUP on an unbindable mount Christian Brauner
@ 2026-09-23 12:27 ` Christian Brauner
2026-09-23 12:27 ` [PATCH 5/8] fs: don't silently unmount busy mounts Christian Brauner
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2026-09-23 12:27 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Alexander Viro, Jan Kara,
Christian Brauner (Amutable)
MOVE_MOUNT_SET_GROUP must not accept an unbindable mount as the target.
Cover both sources:
- a slave, which used to leave the target unbindable and a slave at once
- a shared mount, which used to silently drop the unbindable flag
Check that the target is untouched after the refused call.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
.../move_mount_set_group_test.c | 74 ++++++++++++++++++++--
1 file changed, 68 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/move_mount_set_group/move_mount_set_group_test.c b/tools/testing/selftests/move_mount_set_group/move_mount_set_group_test.c
index 12434415ec36..9c8fc8c7f62c 100644
--- a/tools/testing/selftests/move_mount_set_group/move_mount_set_group_test.c
+++ b/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
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/8] fs: don't silently unmount busy mounts
2026-09-23 12:27 [PATCH 0/8] mount: a few gnarly fixes Christian Brauner
` (3 preceding siblings ...)
2026-09-23 12:27 ` [PATCH 4/8] selftests/move_mount_set_group: check that an unbindable target is refused Christian Brauner
@ 2026-09-23 12:27 ` Christian Brauner
2026-09-23 12:27 ` [PATCH 6/8] selftests/filesystems: check that a busy propagated copy blocks a synchronous umount Christian Brauner
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2026-09-23 12:27 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Alexander Viro, Jan Kara,
Christian Brauner (Amutable), stable
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/8] selftests/filesystems: check that a busy propagated copy blocks a synchronous umount
2026-09-23 12:27 [PATCH 0/8] mount: a few gnarly fixes Christian Brauner
` (4 preceding siblings ...)
2026-09-23 12:27 ` [PATCH 5/8] fs: don't silently unmount busy mounts Christian Brauner
@ 2026-09-23 12:27 ` Christian Brauner
2026-09-23 12:27 ` [PATCH 7/8] fs: don't let a migrating task hide its reference from do_umount() Christian Brauner
2026-09-23 12:28 ` [PATCH 8/8] docs: update the unmount propagation rule Christian Brauner
7 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2026-09-23 12:27 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Alexander Viro, Jan Kara,
Christian Brauner (Amutable)
A slave namespace moves an open_tree() copy of the shared tree beneath
the propagated copy of the victim and keeps the descriptor. Check that:
- umount(2) of the victim fails with EBUSY while the descriptor is open
- the moved tree is still attached in the slave namespace afterwards
- the umount succeeds once the descriptor is closed
The test needs CAP_SYS_ADMIN and skips otherwise.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
tools/testing/selftests/Makefile | 1 +
.../filesystems/umount_propagation/Makefile | 6 +
.../umount_propagation/umount_propagation_test.c | 226 +++++++++++++++++++++
3 files changed, 233 insertions(+)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index a3df9a15ebb7..43d4a33afe71 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -51,6 +51,7 @@ 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
diff --git a/tools/testing/selftests/filesystems/umount_propagation/Makefile b/tools/testing/selftests/filesystems/umount_propagation/Makefile
new file mode 100644
index 000000000000..fc0a0783018b
--- /dev/null
+++ b/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
diff --git a/tools/testing/selftests/filesystems/umount_propagation/umount_propagation_test.c b/tools/testing/selftests/filesystems/umount_propagation/umount_propagation_test.c
new file mode 100644
index 000000000000..9e18d54dfb32
--- /dev/null
+++ b/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
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 7/8] fs: don't let a migrating task hide its reference from do_umount()
2026-09-23 12:27 [PATCH 0/8] mount: a few gnarly fixes Christian Brauner
` (5 preceding siblings ...)
2026-09-23 12:27 ` [PATCH 6/8] selftests/filesystems: check that a busy propagated copy blocks a synchronous umount Christian Brauner
@ 2026-09-23 12:27 ` Christian Brauner
2026-09-23 12:28 ` [PATCH 8/8] docs: update the unmount propagation rule Christian Brauner
7 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2026-09-23 12:27 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Alexander Viro, Jan Kara,
Christian Brauner (Amutable)
propagate_mount_busy() uses mnt_get_count() via do_refcount_check() to
figure out whether a synchronous umount may proceed or not by summing
the per-CPU mnt_count counter under mount_lock.
The fastpath for mntget() and mntput() doesn't require that lock. A task
that already holds a reference and takes another one on a cpu the loop
has summed can get migrated and drops that reference again on another
cpu. The loop might not have reached that cpu yet. That ends up
undercounting. The original reference of the task is missed and umount()
ends up succeeding with the file still open. The mount is destroyed
later when the task mntput()s so it degrades to a lazy umount.
That's basically the umount(2) variant of commit 9ea0a46ca2c3 ("fix
mntput/mntput race") closed for the final mntput(). The same
cpu-migration caused a UAF.
Luckily this isn't a UAF. It's just a hidden reference pinning the
mount but it's wrong nonetheless.
Like the other race it's a narrow window but I can reproduce it reliably
with some patching. The debug patch below prints the sum the loop
computed next to an immediate re-sum of the same counters. It reports
who drops the last reference to that mount later:
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -34,6 +34,8 @@
#include <linux/mnt_idmapping.h>
#include <linux/pidfs.h>
#include <linux/nstree.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
#include "pnode.h"
#include "internal.h"
@@ -263,17 +265,48 @@
#endif
}
+static u32 mnt_get_count_delay_ms;
+static int mnt_get_count_debug_id;
+
+static int __init mnt_get_count_debug_init(void)
+{
+ debugfs_create_u32("mnt_get_count_delay_ms", 0600, NULL,
+ &mnt_get_count_delay_ms);
+ return 0;
+}
+late_initcall(mnt_get_count_debug_init);
+
/*
* vfsmount lock must be held for write
*/
int mnt_get_count(struct mount *mnt)
{
#ifdef CONFIG_SMP
- int count = 0;
+ static int seen[NR_CPUS];
+ bool slow = mnt_get_count_delay_ms &&
+ !(mnt->mnt.mnt_flags & MNT_UMOUNT);
+ int count = 0, again = 0;
int cpu;
for_each_possible_cpu(cpu) {
- count += per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_count;
+ seen[cpu] = per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_count;
+ count += seen[cpu];
+ if (slow)
+ mdelay(mnt_get_count_delay_ms);
+ }
+ if (slow) {
+ for_each_possible_cpu(cpu) {
+ int now;
+
+ now = per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_count;
+ again += now;
+ pr_err("mount %d: cpu%d read %d, now %d\n",
+ mnt->mnt_id, cpu, seen[cpu], now);
+ }
+ mnt_get_count_debug_id = mnt->mnt_id;
+ pr_err("mount %d: sum %d, re-sum %d, %ps\n",
+ mnt->mnt_id, count, again,
+ __builtin_return_address(0));
}
return count;
@@ -1350,6 +1383,9 @@
smp_mb();
mnt_add_count(mnt, -1);
count = mnt_get_count(mnt);
+ if (count == 0 && mnt->mnt_id == mnt_get_count_debug_id)
+ pr_err("mount %d: last put by %s (%d)\n", mnt->mnt_id,
+ current->comm, task_pid_nr(current));
if (count != 0) {
WARN_ON(count < 0);
rcu_read_unlock();
A task holds open a directory fd on a tmpfs. A synchronous rumount() of
the tmpfs sums up. Concurrently a task calls fchdir() on that fd on
CPU 0 and fchdir()s back to / on CPU 3. This will cause a path_get() and
a path_put() with a migration in between without mount_lock held.
Adding a delay per CPU makes the count loop miss the get in a cpu it
already visited but count the put in one it hasn't.
The reproducer, run as root with debugfs mounted:
/*
* gcc -static -o umount-torn umount-torn.c
*/
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <sched.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/mount.h>
#include <sys/stat.h>
#include <sys/wait.h>
#define KNOB "/sys/kernel/debug/mnt_get_count_delay_ms"
#define MNT "/mnt/x"
static int kmsg;
static void pin(int cpu)
{
cpu_set_t s;
CPU_ZERO(&s);
CPU_SET(cpu, &s);
sched_setaffinity(0, sizeof(s), &s);
}
static void knob(const char *v)
{
int fd = open(KNOB, O_WRONLY);
write(fd, v, strlen(v));
close(fd);
}
int main(void)
{
int last = sysconf(_SC_NPROCESSORS_ONLN) - 1;
int p[2], fd, ret;
pid_t pid;
char c;
kmsg = open("/dev/kmsg", O_WRONLY);
mkdir(MNT, 0755);
mount("none", MNT, "tmpfs", 0, NULL);
fd = open(MNT "/f", O_WRONLY | O_CREAT, 0644);
write(fd, "still here\n", 11);
close(fd);
pipe(p);
pid = fork();
if (pid == 0) {
int dirfd = open(MNT, O_RDONLY | O_DIRECTORY);
int rootfd = open("/", O_RDONLY | O_DIRECTORY);
struct stat st;
char buf[16];
/* one get/put pair while the umount is summing */
usleep(150000);
pin(0);
fchdir(dirfd); /* mntget() */
dprintf(kmsg, "holder %d: get on cpu %d\n", getpid(),
sched_getcpu());
pin(last);
fchdir(rootfd); /* mntput() */
dprintf(kmsg, "holder %d: put on cpu %d\n", getpid(),
sched_getcpu());
/* the umount has returned, is the fd still good? */
read(p[0], &c, 1);
fd = openat(dirfd, "f", O_RDONLY);
ret = read(fd, buf, sizeof(buf));
dprintf(kmsg, "holder %d: fstat %d openat %d read %d\n",
getpid(), fstat(dirfd, &st), fd, ret);
_exit(0);
}
pin(1);
knob("100");
ret = umount2(MNT, 0);
knob("0");
if (ret)
dprintf(kmsg, "umount: %s\n", strerror(errno));
else
dprintf(kmsg, "umount: 0\n");
write(p[1], "x", 1);
waitpid(pid, NULL, 0);
return 0;
}
The log shows the busy check summing 2 and when read right after it
reads 3. The loop read CPU 0 before the get landed there and CPU 3
after the put did. The umount succeeds while there's still someone
with an fd. The mount is then dropped when the fd goes:
[ 2.511944] holder 90: get on cpu 0
[ 2.512023] holder 90: put on cpu 3
[ 2.671388] mount 28: cpu0 read 1, now 2
[ 2.671398] mount 28: cpu1 read 2, now 2
[ 2.671401] mount 28: cpu2 read 0, now 0
[ 2.671404] mount 28: cpu3 read -1, now -1
[ 2.671407] mount 28: sum 2, re-sum 3, propagate_mount_busy
[ 2.671773] umount: 0
[ 2.672963] holder 90: fstat 0 openat 8 read 11
[ 2.673663] mount 28: last put by umount-torn (90)
Easy fix would be to order mntget() against the loop that sums up. But
that's a full barrier and a flag test in mntget() afaict. No bueno...
That would cost every path_get().
So learn from SRCU. Keep gets and puts in separate per-cpu counters and
make mnt_get_count() sum all puts first and all gets second with a full
barrier in between.
A put is always after its own get (hopefully...) independent of the cpu
the task is on. So a put the first pass counted must have its get
visible to the second pass. Ergo, we can't undercount.
This is the two pass sum srcu_readers_active_idx_check() does for
srcu_locks and srcu_unlocks. mntget() keeps a single
this_cpu_inc(). mntput() gains an smp_wmb() before its increment, a
compiler barrier on x86 and a store fence on the weakly ordered
architectures, since the two counters are two words and a put stored
after a get could otherwise become visible before it. The counters
are unsigned so the difference stays right after they wrap.
The count can still come out high when a get is counted whose put
the first pass did not see. We don't care. A task can only take a new
reference if:
(1) it holds one already
(2) through __legitimize_mnt() which the seqcount orders
against the write side
So a get in this window means someone has a reference and -EBUSY is
correct. The final mntput() is unchanged. Once ->mnt_ns is NULL every
put takes the slow path under mount_lock and so no put is in flight
while it counts. Here's an attempted proof for that crap:
(i) A put never becomes visible before its get. Within one task the
smp_wmb() in mntput_no_expire() orders every earlier store before the
increment of the puts.
Any task that migrated in between had its stores flushed by the
scheduler. When that reference is handed to another task that's
trivially true.
(ii) The double count in mnt_get_count() is ordered via smp_mb(). That
keeps every read of the puts counter before every read of the gets
counter.
(iii) We never undercount by (ii).
(iv) There are no spurious counts/differences. A get is counted only if
it happened and a put is left out only if it hasn't happened when
the counter was read. With (iii) this means it's equivalent to the
old count whenever nothing is taken or dropped during the loop.
(v) do_umount() holds a reference and mount_lock hence. New references
come from: (v.a) __legitimize_mnt() managed by the seqcount. Either
it's smp_mb() made the get visible to the loop or it takes the lock
and waits for the loop. By (iii) a mount with a additional
references is refused. By (iv) refusing the umount just means that
any additional reference existed, same as the old counter.
(vi) mntput_no_expire_slowpath() runs with ->mnt_ns already NULL. Hence,
every other put must take the slowpath as well and hangs on mount_lock.
The grace period in namespace_unlock() will make every fast path do
a put that still saw ->mnt_ns.
No put is in flight during the loop.
A get that is in flight comes from a task holding another reference.
That must be included by the count or from __legitimize_mnt() as in
(v).
Ergo, zero means the last reference and non-zero means a reference
is live.
(vii) may_umount() uses propagate_mount_busy() as well and
may_umount_tree() sums mnt_get_count() itself.
With this patch the race is dead:
[ 3.066786] mount 28: sum 3, re-sum 3, propagate_mount_busy
[ 3.105867] umount: Device or resource busy
[ 3.106047] holder 90: fstat 0 openat 8 read 11
Fixes: b3e19d924b6e ("fs: scale mntget/mntput")
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/mount.h | 3 ++-
fs/namespace.c | 48 ++++++++++++++++++++++++++++++------------------
2 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/fs/mount.h b/fs/mount.h
index 94fcc306d21e..85f136786bbc 100644
--- a/fs/mount.h
+++ b/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;
};
diff --git a/fs/namespace.c b/fs/namespace.c
index d842fc1f1f1a..73215f84e04d 100644
--- a/fs/namespace.c
+++ b/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_inc(mnt->mnt_pcp->mnt_gets);
+#else
+ preempt_disable();
+ mnt->mnt_count++;
+ preempt_enable();
+#endif
+}
+
+static inline void mnt_dec_count(struct mount *mnt)
{
#ifdef CONFIG_SMP
- this_cpu_add(mnt->mnt_pcp->mnt_count, n);
+ this_cpu_inc(mnt->mnt_pcp->mnt_puts);
#else
preempt_disable();
- mnt->mnt_count += n;
+ 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;
}
@@ -1348,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);
@@ -1405,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;
}
@@ -1427,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);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 8/8] docs: update the unmount propagation rule
2026-09-23 12:27 [PATCH 0/8] mount: a few gnarly fixes Christian Brauner
` (6 preceding siblings ...)
2026-09-23 12:27 ` [PATCH 7/8] fs: don't let a migrating task hide its reference from do_umount() Christian Brauner
@ 2026-09-23 12:28 ` Christian Brauner
7 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2026-09-23 12:28 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Alexander Viro, Jan Kara,
Christian Brauner (Amutable)
Section 5f still describes the rule propagate_umount() used before
commit f0d0ba19985d ("Rewrite of propagate_umount()"). It states that a
mount that receives the unmount by propagation is left alone as soon as
it has any submount. That's not true anymore.
A propagated unmount unmounts a mount with sub-mounts as long as every
sub-mount gets unmounted together with it. This is the case when the
sub-mounts are unmounted by the same propagation. Only a sub-mount that
cannot be unmounted keeps its parent mounted.
The section also only describes a single mount without sub-mounts. But
lazy unmounts take a tree and every mount of the tree propagates its
unmount from its parent mount.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Documentation/filesystems/sharedsubtree.rst | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/Documentation/filesystems/sharedsubtree.rst b/Documentation/filesystems/sharedsubtree.rst
index 8b7dc9159083..8bae6d9a7e04 100644
--- a/Documentation/filesystems/sharedsubtree.rst
+++ b/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
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread