From 1ab8904ae61b076d1242e1f48537698c77074b5c Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 23 Sep 2026 14:27:53 +0200 Subject: [PATCH 1/8] mount: keep a copied mount unbindable 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. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-1-f424cf8d3242@kernel.org Fixes: 406fea799925 ("mount: separate the flags accessed only under namespace_sem") Cc: stable@vger.kernel.org # v6.17+ Signed-off-by: Christian Brauner (Amutable) --- 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 */ From 0bab6b6d77607458a25710a42c42ae2343468d2c Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 23 Sep 2026 14:27:54 +0200 Subject: [PATCH 2/8] selftests/filesystems: check that a copied mount namespace keeps unbindable 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. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-2-f424cf8d3242@kernel.org Signed-off-by: Christian Brauner (Amutable) --- tools/testing/selftests/Makefile | 1 + .../filesystems/mntns_unbindable/Makefile | 6 + .../mntns_unbindable/mntns_unbindable_test.c | 227 ++++++++++++++++++ 3 files changed, 234 insertions(+) create mode 100644 tools/testing/selftests/filesystems/mntns_unbindable/Makefile create mode 100644 tools/testing/selftests/filesystems/mntns_unbindable/mntns_unbindable_test.c 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#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 From a2b42babc912e4fb428cbb42adb4f6e1e067113d Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 23 Sep 2026 14:27:55 +0200 Subject: [PATCH 3/8] mount: refuse MOVE_MOUNT_SET_GROUP on an unbindable mount 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. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-3-f424cf8d3242@kernel.org Fixes: 9ffb14ef61ba ("move_mount: allow to add a mount into an existing group") Signed-off-by: Christian Brauner (Amutable) --- 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 */ From 7541865c260f9bef9fecf212ec39538a21daaf77 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 23 Sep 2026 14:27:56 +0200 Subject: [PATCH 4/8] selftests/move_mount_set_group: check that an unbindable target is refused 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. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-4-f424cf8d3242@kernel.org Signed-off-by: Christian Brauner (Amutable) --- .../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 From 6857e209015f95be7a8f19a9cd5dac345b9b9859 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 23 Sep 2026 14:27:57 +0200 Subject: [PATCH 5/8] fs: don't silently unmount busy mounts 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. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-5-f424cf8d3242@kernel.org Fixes: 99b19d16471e ("mnt: In propgate_umount handle visiting mounts in any order") Cc: stable@vger.kernel.org Signed-off-by: Christian Brauner (Amutable) --- 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; From bba9960c073dbea8eaa31e9a6b9f4224ebdb6ec8 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 23 Sep 2026 14:27:58 +0200 Subject: [PATCH 6/8] selftests/filesystems: check that a busy propagated copy blocks a synchronous umount 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. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-6-f424cf8d3242@kernel.org Signed-off-by: Christian Brauner (Amutable) --- tools/testing/selftests/Makefile | 1 + .../filesystems/umount_propagation/Makefile | 6 + .../umount_propagation_test.c | 226 ++++++++++++++++++ 3 files changed, 233 insertions(+) create mode 100644 tools/testing/selftests/filesystems/umount_propagation/Makefile create mode 100644 tools/testing/selftests/filesystems/umount_propagation/umount_propagation_test.c 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#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 From 7eb84d54fac5002bd975d1de8fd61d57e89172ab Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 23 Sep 2026 14:27:59 +0200 Subject: [PATCH 7/8] fs: don't let a migrating task hide its reference from do_umount() 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 #include #include +#include +#include #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 #include #include #include #include #include #include #include #include #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 Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-7-f424cf8d3242@kernel.org Fixes: b3e19d924b6e ("fs: scale mntget/mntput") Signed-off-by: Christian Brauner (Amutable) --- 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_add(mnt->mnt_pcp->mnt_count, n); + this_cpu_inc(mnt->mnt_pcp->mnt_gets); #else preempt_disable(); - mnt->mnt_count += n; + mnt->mnt_count++; + preempt_enable(); +#endif +} + +static inline void mnt_dec_count(struct mount *mnt) +{ +#ifdef CONFIG_SMP + this_cpu_inc(mnt->mnt_pcp->mnt_puts); +#else + preempt_disable(); + mnt->mnt_count--; preempt_enable(); #endif } @@ -269,14 +277,17 @@ static inline void mnt_add_count(struct mount *mnt, int n) int mnt_get_count(struct mount *mnt) { #ifdef CONFIG_SMP - int count = 0; + unsigned int gets = 0, puts = 0; int cpu; - for_each_possible_cpu(cpu) { - count += per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_count; - } + /* puts first, so a put counted here has its get counted below */ + for_each_possible_cpu(cpu) + puts += per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_puts; + smp_mb(); /* pairs with the smp_wmb() in mntput_no_expire() */ + for_each_possible_cpu(cpu) + gets += per_cpu_ptr(mnt->mnt_pcp, cpu)->mnt_gets; - return count; + return gets - puts; #else return mnt->mnt_count; #endif @@ -305,7 +316,7 @@ static struct mount *alloc_vfsmnt(const char *name) if (!mnt->mnt_pcp) goto out_free_devname; - this_cpu_add(mnt->mnt_pcp->mnt_count, 1); + this_cpu_inc(mnt->mnt_pcp->mnt_gets); #else mnt->mnt_count = 1; mnt->mnt_writers = 0; @@ -746,13 +757,13 @@ int __legitimize_mnt(struct vfsmount *bastard, unsigned seq) if (bastard == NULL) return 0; mnt = real_mount(bastard); - mnt_add_count(mnt, 1); - smp_mb(); // see mntput_no_expire() and do_umount() + mnt_inc_count(mnt); + smp_mb(); /* see mntput_no_expire_slowpath() and do_umount() */ if (likely(!read_seqretry(&mount_lock, seq))) return 0; lock_mount_hash(); if (unlikely(bastard->mnt_flags & (MNT_SYNC_UMOUNT | MNT_DOOMED))) { - mnt_add_count(mnt, -1); + mnt_dec_count(mnt); unlock_mount_hash(); return 1; } @@ -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); From 3f5dc09141a7bfe286620a6869d1d70ffe32d6b8 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 23 Sep 2026 14:28:00 +0200 Subject: [PATCH 8/8] docs: update the unmount propagation rule 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. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-8-f424cf8d3242@kernel.org Signed-off-by: Christian Brauner (Amutable) --- 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