From 8c6ce8e86dd75db8e6c6a3e5a870e8d52dbab2d0 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 12:40:28 -0400 Subject: attach_mnt(): expand in attach_recursive_mnt(), then lose the flag argument simpler that way - all but one caller pass false as 'beneath' argument, and that one caller is actually happier with the call expanded - the logics with choice of mountpoint is identical for 'moving' and 'attaching' cases, and now that is no longer hidden. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 54c59e091919..1761d2c2fdae 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1116,16 +1116,10 @@ static void __attach_mnt(struct mount *mnt, struct mount *parent) * @parent: the parent * @mnt: the new mount * @mp: the new mountpoint - * @beneath: whether to mount @mnt beneath or on top of @parent * - * If @beneath is false, mount @mnt at @mp on @parent. Then attach @mnt + * Mount @mnt at @mp on @parent. Then attach @mnt * to @parent's child mount list and to @mount_hashtable. * - * If @beneath is true, remove @mnt from its current parent and - * mountpoint and mount it on @mp on @parent, and mount @parent on the - * old parent and old mountpoint of @mnt. Finally, attach @parent to - * @mnt_hashtable and @parent->mnt_parent->mnt_mounts. - * * Note, when __attach_mnt() is called @mnt->mnt_parent already points * to the correct parent. * @@ -1133,18 +1127,9 @@ static void __attach_mnt(struct mount *mnt, struct mount *parent) * to have been acquired in that order. */ static void attach_mnt(struct mount *mnt, struct mount *parent, - struct mountpoint *mp, bool beneath) + struct mountpoint *mp) { - if (beneath) - mnt_set_mountpoint_beneath(mnt, parent, mp); - else - mnt_set_mountpoint(parent, mp, mnt); - /* - * Note, @mnt->mnt_parent has to be used. If @mnt was mounted - * beneath @parent then @mnt will need to be attached to - * @parent's old parent, not @parent. IOW, @mnt->mnt_parent - * isn't the same mount as @parent. - */ + mnt_set_mountpoint(parent, mp, mnt); __attach_mnt(mnt, mnt->mnt_parent); } @@ -1157,7 +1142,7 @@ void mnt_change_mountpoint(struct mount *parent, struct mountpoint *mp, struct m hlist_del_init(&mnt->mnt_mp_list); hlist_del_init_rcu(&mnt->mnt_hash); - attach_mnt(mnt, parent, mp, false); + attach_mnt(mnt, parent, mp); put_mountpoint(old_mp); mnt_add_count(old_parent, -1); @@ -2295,7 +2280,7 @@ struct mount *copy_tree(struct mount *src_root, struct dentry *dentry, goto out; lock_mount_hash(); list_add_tail(&dst_mnt->mnt_list, &res->mnt_list); - attach_mnt(dst_mnt, dst_parent, src_parent->mnt_mp, false); + attach_mnt(dst_mnt, dst_parent, src_parent->mnt_mp); unlock_mount_hash(); } } @@ -2743,10 +2728,12 @@ static int attach_recursive_mnt(struct mount *source_mnt, } if (moving) { - if (beneath) - dest_mp = smp; unhash_mnt(source_mnt); - attach_mnt(source_mnt, top_mnt, dest_mp, beneath); + if (beneath) + mnt_set_mountpoint_beneath(source_mnt, top_mnt, smp); + else + mnt_set_mountpoint(top_mnt, dest_mp, source_mnt); + __attach_mnt(source_mnt, source_mnt->mnt_parent); mnt_notify_add(source_mnt); touch_mnt_namespace(source_mnt->mnt_ns); } else { @@ -4827,9 +4814,9 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, root_mnt->mnt.mnt_flags &= ~MNT_LOCKED; } /* mount old root on put_old */ - attach_mnt(root_mnt, old_mnt, old_mp, false); + attach_mnt(root_mnt, old_mnt, old_mp); /* mount new_root on / */ - attach_mnt(new_mnt, root_parent, root_mp, false); + attach_mnt(new_mnt, root_parent, root_mp); mnt_add_count(root_parent, -1); touch_mnt_namespace(current->nsproxy->mnt_ns); /* A moved mount should not expire automatically */ -- cgit v1.2.3 From 431cc1d8e2dab751b2b8f298a6d9caf83d8b49c3 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 22:18:12 -0400 Subject: get rid of mnt_set_mountpoint_beneath() mnt_set_mountpoint_beneath() consists of attaching new mount side-by-side with the one we want to mount beneath (by mnt_set_mountpoint()), followed by mnt_change_mountpoint() shifting the the top mount onto the new one (by mnt_change_mountpoint()). Both callers of mnt_set_mountpoint_beneath (both in attach_recursive_mnt()) have the same form - in 'beneath' case we call mnt_set_mountpoint_beneath(), otherwise - mnt_set_mountpoint(). The thing is, expressing that as unconditional mnt_set_mountpoint(), followed, in 'beneath' case, by mnt_change_mountpoint() is just as easy. And these mnt_change_mountpoint() callers are similar to the ones we do when it comes to attaching propagated copies, which will allow more cleanups in the next commits. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 37 ++++--------------------------------- 1 file changed, 4 insertions(+), 33 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 1761d2c2fdae..888816289154 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1076,33 +1076,6 @@ void mnt_set_mountpoint(struct mount *mnt, hlist_add_head(&child_mnt->mnt_mp_list, &mp->m_list); } -/** - * mnt_set_mountpoint_beneath - mount a mount beneath another one - * - * @new_parent: the source mount - * @top_mnt: the mount beneath which @new_parent is mounted - * @new_mp: the new mountpoint of @top_mnt on @new_parent - * - * Remove @top_mnt from its current mountpoint @top_mnt->mnt_mp and - * parent @top_mnt->mnt_parent and mount it on top of @new_parent at - * @new_mp. And mount @new_parent on the old parent and old - * mountpoint of @top_mnt. - * - * Context: This function expects namespace_lock() and lock_mount_hash() - * to have been acquired in that order. - */ -static void mnt_set_mountpoint_beneath(struct mount *new_parent, - struct mount *top_mnt, - struct mountpoint *new_mp) -{ - struct mount *old_top_parent = top_mnt->mnt_parent; - struct mountpoint *old_top_mp = top_mnt->mnt_mp; - - mnt_set_mountpoint(old_top_parent, old_top_mp, new_parent); - mnt_change_mountpoint(new_parent, new_mp, top_mnt); -} - - static void __attach_mnt(struct mount *mnt, struct mount *parent) { hlist_add_head_rcu(&mnt->mnt_hash, @@ -2729,10 +2702,9 @@ static int attach_recursive_mnt(struct mount *source_mnt, if (moving) { unhash_mnt(source_mnt); + mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); if (beneath) - mnt_set_mountpoint_beneath(source_mnt, top_mnt, smp); - else - mnt_set_mountpoint(top_mnt, dest_mp, source_mnt); + mnt_change_mountpoint(source_mnt, smp, top_mnt); __attach_mnt(source_mnt, source_mnt->mnt_parent); mnt_notify_add(source_mnt); touch_mnt_namespace(source_mnt->mnt_ns); @@ -2745,10 +2717,9 @@ static int attach_recursive_mnt(struct mount *source_mnt, move_from_ns(p, &head); list_del_init(&head); } + mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); if (beneath) - mnt_set_mountpoint_beneath(source_mnt, top_mnt, smp); - else - mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); + mnt_change_mountpoint(source_mnt, smp, top_mnt); commit_tree(source_mnt); } -- cgit v1.2.3 From ffdc52fbbd5835a936ad683c943d6d103a2d4514 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 20 Jun 2025 22:46:55 -0400 Subject: prevent mount hash conflicts Currently it's still possible to run into a pathological situation when two hashed mounts share both parent and mountpoint. That does not work well, for obvious reasons. We are not far from getting rid of that; the only remaining gap is attach_recursive_mnt() not being careful enough when sliding a tree under existing mount (for propagated copies or in 'beneath' case for the original one). To deal with that cleanly we need to be able to find overmounts (i.e. mounts on top of parent's root); we could do hash lookups or scan the list of children but either would be costly. Since one of the results we get from that will be prevention of multiple parallel overmounts, let's just bite the bullet and store a (non-counting) reference to overmount in struct mount. With that done, closing the hole in attach_recursive_mnt() becomes easy - we just need to follow the chain of overmounts before we change the mountpoint of the mount we are sliding things under. Signed-off-by: Al Viro --- fs/mount.h | 1 + fs/namespace.c | 27 ++++++++++++++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index ad7173037924..b8beafdd6d24 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -92,6 +92,7 @@ struct mount { int mnt_expiry_mark; /* true if marked for expiry */ struct hlist_head mnt_pins; struct hlist_head mnt_stuck_children; + struct mount *overmount; /* mounted on ->mnt_root */ } __randomize_layout; #define MNT_NS_INTERNAL ERR_PTR(-EINVAL) /* distinct from any mnt_namespace */ diff --git a/fs/namespace.c b/fs/namespace.c index 888816289154..9b732d74c2cc 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1043,6 +1043,9 @@ static void __touch_mnt_namespace(struct mnt_namespace *ns) static struct mountpoint *unhash_mnt(struct mount *mnt) { struct mountpoint *mp; + struct mount *parent = mnt->mnt_parent; + if (unlikely(parent->overmount == mnt)) + parent->overmount = NULL; mnt->mnt_parent = mnt; mnt->mnt_mountpoint = mnt->mnt.mnt_root; list_del_init(&mnt->mnt_child); @@ -1078,6 +1081,8 @@ void mnt_set_mountpoint(struct mount *mnt, static void __attach_mnt(struct mount *mnt, struct mount *parent) { + if (unlikely(mnt->mnt_mountpoint == parent->mnt.mnt_root)) + parent->overmount = mnt; hlist_add_head_rcu(&mnt->mnt_hash, m_hash(&parent->mnt, mnt->mnt_mountpoint)); list_add_tail(&mnt->mnt_child, &parent->mnt_mounts); @@ -2660,7 +2665,9 @@ static int attach_recursive_mnt(struct mount *source_mnt, HLIST_HEAD(tree_list); struct mnt_namespace *ns = top_mnt->mnt_ns; struct mountpoint *smp; + struct mountpoint *secondary = NULL; struct mount *child, *dest_mnt, *p; + struct mount *top; struct hlist_node *n; int err = 0; bool moving = flags & MNT_TREE_MOVE, beneath = flags & MNT_TREE_BENEATH; @@ -2669,9 +2676,15 @@ static int attach_recursive_mnt(struct mount *source_mnt, * Preallocate a mountpoint in case the new mounts need to be * mounted beneath mounts on the same mountpoint. */ - smp = get_mountpoint(source_mnt->mnt.mnt_root); + for (top = source_mnt; unlikely(top->overmount); top = top->overmount) { + if (!secondary && is_mnt_ns_file(top->mnt.mnt_root)) + secondary = top->mnt_mp; + } + smp = get_mountpoint(top->mnt.mnt_root); if (IS_ERR(smp)) return PTR_ERR(smp); + if (!secondary) + secondary = smp; /* Is there space to add these mounts to the mount namespace? */ if (!moving) { @@ -2704,7 +2717,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, unhash_mnt(source_mnt); mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); if (beneath) - mnt_change_mountpoint(source_mnt, smp, top_mnt); + mnt_change_mountpoint(top, smp, top_mnt); __attach_mnt(source_mnt, source_mnt->mnt_parent); mnt_notify_add(source_mnt); touch_mnt_namespace(source_mnt->mnt_ns); @@ -2719,7 +2732,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, } mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); if (beneath) - mnt_change_mountpoint(source_mnt, smp, top_mnt); + mnt_change_mountpoint(top, smp, top_mnt); commit_tree(source_mnt); } @@ -2732,8 +2745,12 @@ static int attach_recursive_mnt(struct mount *source_mnt, child->mnt.mnt_flags &= ~MNT_LOCKED; q = __lookup_mnt(&child->mnt_parent->mnt, child->mnt_mountpoint); - if (q) - mnt_change_mountpoint(child, smp, q); + if (q) { + struct mount *r = child; + while (unlikely(r->overmount)) + r = r->overmount; + mnt_change_mountpoint(r, secondary, q); + } commit_tree(child); } put_mountpoint(smp); -- cgit v1.2.3 From cf53a2d423c11ed70611e7b3f0878d6e419e348a Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 1 Jun 2025 00:34:32 -0400 Subject: copy_tree(): don't set ->mnt_mountpoint on the root of copy It never made any sense - neither when copy_tree() had been introduced (2.4.11-pre5), nor at any point afterwards. Mountpoint is meaningless without parent mount and the root of copied tree has no parent until we get around to attaching it somewhere. At that time we'll have mountpoint set; before that we have no idea which dentry will be used as mountpoint. IOW, copy_tree() should just leave the default value. Signed-off-by: Al Viro --- fs/namespace.c | 1 - 1 file changed, 1 deletion(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 9b732d74c2cc..f0a56dbceff9 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2222,7 +2222,6 @@ struct mount *copy_tree(struct mount *src_root, struct dentry *dentry, return dst_mnt; src_parent = src_root; - dst_mnt->mnt_mountpoint = src_root->mnt_mountpoint; list_for_each_entry(src_root_child, &src_root->mnt_mounts, mnt_child) { if (!is_subdir(src_root_child->mnt_mountpoint, dentry)) -- cgit v1.2.3 From 0e84653ea596bf9f5bfea58b0a34e0d9f72236c4 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 28 Apr 2025 21:48:45 -0400 Subject: constify mnt_has_parent() Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/mount.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index b8beafdd6d24..c4d417cd7953 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -102,7 +102,7 @@ static inline struct mount *real_mount(struct vfsmount *mnt) return container_of(mnt, struct mount, mnt); } -static inline int mnt_has_parent(struct mount *mnt) +static inline int mnt_has_parent(const struct mount *mnt) { return mnt != mnt->mnt_parent; } -- cgit v1.2.3 From 592238c03ef9e44ad6031b671da869ebcbffa8dc Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 8 May 2025 17:28:00 -0400 Subject: pnode: lift peers() into pnode.h it's going to be useful both in pnode.c and namespace.c Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/pnode.c | 5 ----- fs/pnode.h | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index ffd429b760d5..aa187144e389 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -216,11 +216,6 @@ static struct mount *next_group(struct mount *m, struct mount *origin) static struct mount *last_dest, *first_source, *last_source, *dest_master; static struct hlist_head *list; -static inline bool peers(const struct mount *m1, const struct mount *m2) -{ - return m1->mnt_group_id == m2->mnt_group_id && m1->mnt_group_id; -} - static int propagate_one(struct mount *m, struct mountpoint *dest_mp) { struct mount *child; diff --git a/fs/pnode.h b/fs/pnode.h index 2d026fb98b18..93fa9311bd07 100644 --- a/fs/pnode.h +++ b/fs/pnode.h @@ -34,6 +34,11 @@ static inline void set_mnt_shared(struct mount *mnt) mnt->mnt.mnt_flags |= MNT_SHARED; } +static inline bool peers(const struct mount *m1, const struct mount *m2) +{ + return m1->mnt_group_id == m2->mnt_group_id && m1->mnt_group_id; +} + void change_mnt_propagation(struct mount *, int); int propagate_mnt(struct mount *, struct mountpoint *, struct mount *, struct hlist_head *); -- cgit v1.2.3 From 9cb79ed60e38a0767b5b94c31bf1abf6518bc6b9 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 8 Jun 2025 23:10:33 -0400 Subject: new predicate: mount_is_ancestor() mount_is_ancestor(p1, p2) returns true iff there is a possibly empty ancestry chain from p1 to p2. Convert the open-coded checks. Unlike those open-coded variants it does not depend upon p1 not being root... Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index f0a56dbceff9..aa93e1a48b5d 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -3483,6 +3483,17 @@ static inline bool path_overmounted(const struct path *path) return unlikely(!no_child); } +/* + * Check if there is a possibly empty chain of descent from p1 to p2. + * Locks: namespace_sem (shared) or mount_lock (read_seqlock_excl). + */ +static bool mount_is_ancestor(const struct mount *p1, const struct mount *p2) +{ + while (p2 != p1 && mnt_has_parent(p2)) + p2 = p2->mnt_parent; + return p2 == p1; +} + /** * can_move_mount_beneath - check that we can mount beneath the top mount * @from: mount to mount beneath @@ -3534,9 +3545,8 @@ static int can_move_mount_beneath(const struct path *from, if (parent_mnt_to == current->nsproxy->mnt_ns->root) return -EINVAL; - for (struct mount *p = mnt_from; mnt_has_parent(p); p = p->mnt_parent) - if (p == mnt_to) - return -EINVAL; + if (mount_is_ancestor(mnt_to, mnt_from)) + return -EINVAL; /* * If the parent mount propagates to the child mount this would @@ -3705,9 +3715,8 @@ static int do_move_mount(struct path *old_path, err = -ELOOP; if (!check_for_nsfs_mounts(old)) goto out; - for (; mnt_has_parent(p); p = p->mnt_parent) - if (p == old) - goto out; + if (mount_is_ancestor(old, p)) + goto out; err = attach_recursive_mnt(old, real_mount(new_path->mnt), mp, flags); if (err) -- cgit v1.2.3 From e031251cb249f824ad67cb0b2fc18b68d5792b8d Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 9 Jun 2025 22:03:17 -0400 Subject: constify is_local_mountpoint() Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/mount.h | 4 ++-- fs/namespace.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index c4d417cd7953..f10776003643 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -147,8 +147,8 @@ struct proc_mounts { extern const struct seq_operations mounts_op; -extern bool __is_local_mountpoint(struct dentry *dentry); -static inline bool is_local_mountpoint(struct dentry *dentry) +extern bool __is_local_mountpoint(const struct dentry *dentry); +static inline bool is_local_mountpoint(const struct dentry *dentry) { if (!d_mountpoint(dentry)) return false; diff --git a/fs/namespace.c b/fs/namespace.c index aa93e1a48b5d..c4feb8315978 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -894,7 +894,7 @@ struct vfsmount *lookup_mnt(const struct path *path) * namespace not just a mount that happens to have some specified * parent mount. */ -bool __is_local_mountpoint(struct dentry *dentry) +bool __is_local_mountpoint(const struct dentry *dentry) { struct mnt_namespace *ns = current->nsproxy->mnt_ns; struct mount *mnt, *n; -- cgit v1.2.3 From 05da054d43770e229dfb0e185c15452eed14364c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 8 Jun 2025 23:25:36 -0400 Subject: new predicate: anon_ns_root(mount) checks if mount is the root of an anonymouns namespace. Switch open-coded equivalents to using it. For mounts that belong to anon namespace !mnt_has_parent(mount) is the same as mount == ns->root, and intent is more obvious in the latter form. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/mount.h | 7 +++++++ fs/namespace.c | 28 +++------------------------- 2 files changed, 10 insertions(+), 25 deletions(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index f10776003643..f20e6ed845fe 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -161,6 +161,13 @@ static inline bool is_anon_ns(struct mnt_namespace *ns) return ns->seq == 0; } +static inline bool anon_ns_root(const struct mount *m) +{ + struct mnt_namespace *ns = READ_ONCE(m->mnt_ns); + + return !IS_ERR_OR_NULL(ns) && is_anon_ns(ns) && m == ns->root; +} + static inline bool mnt_ns_attached(const struct mount *mnt) { return !RB_EMPTY_NODE(&mnt->mnt_node); diff --git a/fs/namespace.c b/fs/namespace.c index c4feb8315978..ea01fea2ac93 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2480,9 +2480,7 @@ struct vfsmount *clone_private_mount(const struct path *path) * loops get created. */ if (!check_mnt(old_mnt)) { - if (!is_mounted(&old_mnt->mnt) || - !is_anon_ns(old_mnt->mnt_ns) || - mnt_has_parent(old_mnt)) + if (!anon_ns_root(old_mnt)) return ERR_PTR(-EINVAL); if (!check_for_nsfs_mounts(old_mnt)) @@ -3649,9 +3647,6 @@ static int do_move_mount(struct path *old_path, ns = old->mnt_ns; err = -EINVAL; - /* The thing moved must be mounted... */ - if (!is_mounted(&old->mnt)) - goto out; if (check_mnt(old)) { /* if the source is in our namespace... */ @@ -3664,10 +3659,8 @@ static int do_move_mount(struct path *old_path, } else { /* * otherwise the source must be the root of some anon namespace. - * AV: check for mount being root of an anon namespace is worth - * an inlined predicate... */ - if (!is_anon_ns(ns) || mnt_has_parent(old)) + if (!anon_ns_root(old)) goto out; /* * Bail out early if the target is within the same namespace - @@ -5028,22 +5021,7 @@ static int do_mount_setattr(struct path *path, struct mount_kattr *kattr) err = -EINVAL; lock_mount_hash(); - /* Ensure that this isn't anything purely vfs internal. */ - if (!is_mounted(&mnt->mnt)) - goto out; - - /* - * If this is an attached mount make sure it's located in the callers - * mount namespace. If it's not don't let the caller interact with it. - * - * If this mount doesn't have a parent it's most often simply a - * detached mount with an anonymous mount namespace. IOW, something - * that's simply not attached yet. But there are apparently also users - * that do change mount properties on the rootfs itself. That obviously - * neither has a parent nor is it a detached mount so we cannot - * unconditionally check for detached mounts. - */ - if ((mnt_has_parent(mnt) || !is_anon_ns(mnt->mnt_ns)) && !check_mnt(mnt)) + if (!anon_ns_root(mnt) && !check_mnt(mnt)) goto out; /* -- cgit v1.2.3 From 9ed4b9eaeaa71cb0db4ec8460b5edd390aef58dd Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 8 Jun 2025 23:41:23 -0400 Subject: dissolve_on_fput(): use anon_ns_root() that's the condition we are actually trying to check there... Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 62 ++++++++++++---------------------------------------------- 1 file changed, 13 insertions(+), 49 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index ea01fea2ac93..151d5f3360b9 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2333,67 +2333,31 @@ void drop_collected_paths(struct path *paths, struct path *prealloc) static void free_mnt_ns(struct mnt_namespace *); static struct mnt_namespace *alloc_mnt_ns(struct user_namespace *, bool); -static inline bool must_dissolve(struct mnt_namespace *mnt_ns) -{ - /* - * This mount belonged to an anonymous mount namespace - * but was moved to a non-anonymous mount namespace and - * then unmounted. - */ - if (unlikely(!mnt_ns)) - return false; - - /* - * This mount belongs to a non-anonymous mount namespace - * and we know that such a mount can never transition to - * an anonymous mount namespace again. - */ - if (!is_anon_ns(mnt_ns)) { - /* - * A detached mount either belongs to an anonymous mount - * namespace or a non-anonymous mount namespace. It - * should never belong to something purely internal. - */ - VFS_WARN_ON_ONCE(mnt_ns == MNT_NS_INTERNAL); - return false; - } - - return true; -} - void dissolve_on_fput(struct vfsmount *mnt) { struct mnt_namespace *ns; struct mount *m = real_mount(mnt); + /* + * m used to be the root of anon namespace; if it still is one, + * we need to dissolve the mount tree and free that namespace. + * Let's try to avoid taking namespace_sem if we can determine + * that there's nothing to do without it - rcu_read_lock() is + * enough to make anon_ns_root() memory-safe and once m has + * left its namespace, it's no longer our concern, since it will + * never become a root of anon ns again. + */ + scoped_guard(rcu) { - if (!must_dissolve(READ_ONCE(m->mnt_ns))) + if (!anon_ns_root(m)) return; } scoped_guard(namespace_lock, &namespace_sem) { - ns = m->mnt_ns; - if (!must_dissolve(ns)) - return; - - /* - * After must_dissolve() we know that this is a detached - * mount in an anonymous mount namespace. - * - * Now when mnt_has_parent() reports that this mount - * tree has a parent, we know that this anonymous mount - * tree has been moved to another anonymous mount - * namespace. - * - * So when closing this file we cannot unmount the mount - * tree. This will be done when the file referring to - * the root of the anonymous mount namespace will be - * closed (It could already be closed but it would sync - * on @namespace_sem and wait for us to finish.). - */ - if (mnt_has_parent(m)) + if (!anon_ns_root(m)) return; + ns = m->mnt_ns; lock_mount_hash(); umount_tree(m, UMOUNT_CONNECTED); unlock_mount_hash(); -- cgit v1.2.3 From 1a867d729f951f1f0ef73c94d2f739f959cd8699 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 17 Jun 2025 21:10:02 -0400 Subject: __attach_mnt(): lose the second argument It's always ->mnt_parent of the first one. What the function does is making a mount (with already set parent and mountpoint) visible - in mount hash and in the parent's list of children. IOW, it takes the existing rootwards linkage and sets the matching crownwards linkage. Renamed to make_visible(), while we are at it. Signed-off-by: Al Viro --- fs/namespace.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 151d5f3360b9..75d45d0b615c 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1079,8 +1079,9 @@ void mnt_set_mountpoint(struct mount *mnt, hlist_add_head(&child_mnt->mnt_mp_list, &mp->m_list); } -static void __attach_mnt(struct mount *mnt, struct mount *parent) +static void make_visible(struct mount *mnt) { + struct mount *parent = mnt->mnt_parent; if (unlikely(mnt->mnt_mountpoint == parent->mnt.mnt_root)) parent->overmount = mnt; hlist_add_head_rcu(&mnt->mnt_hash, @@ -1098,7 +1099,7 @@ static void __attach_mnt(struct mount *mnt, struct mount *parent) * Mount @mnt at @mp on @parent. Then attach @mnt * to @parent's child mount list and to @mount_hashtable. * - * Note, when __attach_mnt() is called @mnt->mnt_parent already points + * Note, when make_visible() is called @mnt->mnt_parent already points * to the correct parent. * * Context: This function expects namespace_lock() and lock_mount_hash() @@ -1108,7 +1109,7 @@ static void attach_mnt(struct mount *mnt, struct mount *parent, struct mountpoint *mp) { mnt_set_mountpoint(parent, mp, mnt); - __attach_mnt(mnt, mnt->mnt_parent); + make_visible(mnt); } void mnt_change_mountpoint(struct mount *parent, struct mountpoint *mp, struct mount *mnt) @@ -1182,7 +1183,7 @@ static void commit_tree(struct mount *mnt) n->nr_mounts += n->pending_mounts; n->pending_mounts = 0; - __attach_mnt(mnt, parent); + make_visible(mnt); touch_mnt_namespace(n); } @@ -2679,7 +2680,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); if (beneath) mnt_change_mountpoint(top, smp, top_mnt); - __attach_mnt(source_mnt, source_mnt->mnt_parent); + make_visible(source_mnt); mnt_notify_add(source_mnt); touch_mnt_namespace(source_mnt->mnt_ns); } else { -- cgit v1.2.3 From d08fa7f44ae7f5ad5c842e15f2412790736a6144 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 6 May 2025 18:48:05 -0400 Subject: don't set MNT_LOCKED on parentless mounts Originally MNT_LOCKED meant only one thing - "don't let this mount to be peeled off its parent, we don't want to have its mountpoint exposed". Accordingly, it had only been set on mounts that *do* have a parent. Later it got overloaded with another use - setting it on the absolute root had given free protection against umount(2) of absolute root (was possible to trigger, oopsed). Not a bad trick, but it ended up costing more than it bought us. Unfortunately, the cost included both hard-to-reason-about logics and a subtle race between mount -o remount,ro and mount --[r]bind - lockless &= ~MNT_LOCKED in the end of __do_loopback() could race with sb_prepare_remount_readonly() setting and clearing MNT_HOLD_WRITE (under mount_lock, as it should be). The race wouldn't be much of a problem (there are other ways to deal with it), but the subtlety is. Turns out that nobody except umount(2) had ever made use of having MNT_LOCKED set on absolute root. So let's give up on that trick, clever as it had been, add an explicit check in do_umount() and return to using MNT_LOCKED only for mounts that have a parent. It means that * clone_mnt() no longer copies MNT_LOCKED * copy_tree() sets it on submounts if their counterparts had been marked such, and does that right next to attach_mnt() in there, in the same mount_lock scope. * __do_loopback() no longer needs to strip MNT_LOCKED off the root of subtree it's about to return; no store, no race. * init_mount_tree() doesn't bother setting MNT_LOCKED on absolute root. * lock_mnt_tree() does not set MNT_LOCKED on the subtree's root; accordingly, its caller (loop in attach_recursive_mnt()) does not need to bother stripping that MNT_LOCKED on root. Note that lock_mnt_tree() setting MNT_LOCKED on submounts happens in the same mount_lock scope as __attach_mnt() (from commit_tree()) that makes them reachable. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 75d45d0b615c..791904128f1e 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1313,7 +1313,7 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, } mnt->mnt.mnt_flags = old->mnt.mnt_flags; - mnt->mnt.mnt_flags &= ~(MNT_WRITE_HOLD|MNT_MARKED|MNT_INTERNAL); + mnt->mnt.mnt_flags &= ~(MNT_WRITE_HOLD|MNT_MARKED|MNT_INTERNAL|MNT_LOCKED); atomic_inc(&sb->s_active); mnt->mnt.mnt_idmap = mnt_idmap_get(mnt_idmap(&old->mnt)); @@ -1988,6 +1988,9 @@ static int do_umount(struct mount *mnt, int flags) if (mnt->mnt.mnt_flags & MNT_LOCKED) goto out; + if (!mnt_has_parent(mnt)) /* not the absolute root */ + goto out; + event++; if (flags & MNT_DETACH) { if (mnt_ns_attached(mnt) || !list_empty(&mnt->mnt_list)) @@ -2257,6 +2260,8 @@ struct mount *copy_tree(struct mount *src_root, struct dentry *dentry, if (IS_ERR(dst_mnt)) goto out; lock_mount_hash(); + if (src_mnt->mnt.mnt_flags & MNT_LOCKED) + dst_mnt->mnt.mnt_flags |= MNT_LOCKED; list_add_tail(&dst_mnt->mnt_list, &res->mnt_list); attach_mnt(dst_mnt, dst_parent, src_parent->mnt_mp); unlock_mount_hash(); @@ -2489,7 +2494,7 @@ static void lock_mnt_tree(struct mount *mnt) if (flags & MNT_NOEXEC) flags |= MNT_LOCK_NOEXEC; /* Don't allow unprivileged users to reveal what is under a mount */ - if (list_empty(&p->mnt_expire)) + if (list_empty(&p->mnt_expire) && p != mnt) flags |= MNT_LOCKED; p->mnt.mnt_flags = flags; } @@ -2704,7 +2709,6 @@ static int attach_recursive_mnt(struct mount *source_mnt, /* Notice when we are propagating across user namespaces */ if (child->mnt_parent->mnt_ns->user_ns != user_ns) lock_mnt_tree(child); - child->mnt.mnt_flags &= ~MNT_LOCKED; q = __lookup_mnt(&child->mnt_parent->mnt, child->mnt_mountpoint); if (q) { @@ -2985,26 +2989,21 @@ static inline bool may_copy_tree(struct path *path) static struct mount *__do_loopback(struct path *old_path, int recurse) { - struct mount *mnt = ERR_PTR(-EINVAL), *old = real_mount(old_path->mnt); + struct mount *old = real_mount(old_path->mnt); if (IS_MNT_UNBINDABLE(old)) - return mnt; + return ERR_PTR(-EINVAL); if (!may_copy_tree(old_path)) - return mnt; + return ERR_PTR(-EINVAL); if (!recurse && __has_locked_children(old, old_path->dentry)) - return mnt; + return ERR_PTR(-EINVAL); if (recurse) - mnt = copy_tree(old, old_path->dentry, CL_COPY_MNT_NS_FILE); + return copy_tree(old, old_path->dentry, CL_COPY_MNT_NS_FILE); else - mnt = clone_mnt(old, old_path->dentry, 0); - - if (!IS_ERR(mnt)) - mnt->mnt.mnt_flags &= ~MNT_LOCKED; - - return mnt; + return clone_mnt(old, old_path->dentry, 0); } /* @@ -4749,11 +4748,11 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, if (!path_mounted(&root)) goto out4; /* not a mountpoint */ if (!mnt_has_parent(root_mnt)) - goto out4; /* not attached */ + goto out4; /* absolute root */ if (!path_mounted(&new)) goto out4; /* not a mountpoint */ if (!mnt_has_parent(new_mnt)) - goto out4; /* not attached */ + goto out4; /* absolute root */ /* make sure we can reach put_old from new_root */ if (!is_path_reachable(old_mnt, old.dentry, &new)) goto out4; @@ -6154,7 +6153,6 @@ static void __init init_mount_tree(void) root.mnt = mnt; root.dentry = mnt->mnt_root; - mnt->mnt_flags |= MNT_LOCKED; set_fs_pwd(current->fs, &root); set_fs_root(current->fs, &root); -- cgit v1.2.3 From 49acacdc7cd3c1dcf5190a80ea9e6ca2ffa06cec Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 7 May 2025 14:05:50 -0400 Subject: clone_mnt(): simplify the propagation-related logics The underlying rules are simple: * MNT_SHARED should be set iff ->mnt_group_id of new mount ends up non-zero. * mounts should be on the same ->mnt_share cyclic list iff they have the same non-zero ->mnt_group_id value. * CL_PRIVATE is mutually exclusive with MNT_SHARED, MNT_SLAVE, MNT_SHARED_TO_SLAVE and MNT_EXPIRE; the whole point of that thing is to get a clone of old mount that would *not* be on any namespace-related lists. The above allows to make the logics more straightforward; what's more, it makes the proof that invariants are maintained much simpler. The variant in mainline is safe (aside of a very narrow race with unsafe modification of mnt_flags right after we had the mount exposed in superblock's ->s_mounts; theoretically it can race with ro remount of the original, but it's not easy to hit), but proof of its correctness is really unpleasant. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 791904128f1e..12cf69da4320 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1301,6 +1301,9 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, if (!mnt) return ERR_PTR(-ENOMEM); + mnt->mnt.mnt_flags = READ_ONCE(old->mnt.mnt_flags) & + ~MNT_INTERNAL_FLAGS; + if (flag & (CL_SLAVE | CL_PRIVATE | CL_SHARED_TO_SLAVE)) mnt->mnt_group_id = 0; /* not a peer of original */ else @@ -1312,8 +1315,8 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, goto out_free; } - mnt->mnt.mnt_flags = old->mnt.mnt_flags; - mnt->mnt.mnt_flags &= ~(MNT_WRITE_HOLD|MNT_MARKED|MNT_INTERNAL|MNT_LOCKED); + if (mnt->mnt_group_id) + set_mnt_shared(mnt); atomic_inc(&sb->s_active); mnt->mnt.mnt_idmap = mnt_idmap_get(mnt_idmap(&old->mnt)); @@ -1326,22 +1329,20 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, list_add_tail(&mnt->mnt_instance, &sb->s_mounts); unlock_mount_hash(); + if (flag & CL_PRIVATE) // we are done with it + return mnt; + + if (peers(mnt, old)) + list_add(&mnt->mnt_share, &old->mnt_share); + if ((flag & CL_SLAVE) || ((flag & CL_SHARED_TO_SLAVE) && IS_MNT_SHARED(old))) { list_add(&mnt->mnt_slave, &old->mnt_slave_list); mnt->mnt_master = old; - CLEAR_MNT_SHARED(mnt); - } else if (!(flag & CL_PRIVATE)) { - if ((flag & CL_MAKE_SHARED) || IS_MNT_SHARED(old)) - list_add(&mnt->mnt_share, &old->mnt_share); - if (IS_MNT_SLAVE(old)) - list_add(&mnt->mnt_slave, &old->mnt_slave); + } else if (IS_MNT_SLAVE(old)) { + list_add(&mnt->mnt_slave, &old->mnt_slave); mnt->mnt_master = old->mnt_master; - } else { - CLEAR_MNT_SHARED(mnt); } - if (flag & CL_MAKE_SHARED) - set_mnt_shared(mnt); /* stick the duplicate mount on the same expiry list * as the original if that was on one */ @@ -1349,7 +1350,6 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, if (!list_empty(&old->mnt_expire)) list_add(&mnt->mnt_expire, &old->mnt_expire); } - return mnt; out_free: -- cgit v1.2.3 From c93ff74ff1cbc0ab152cebed3e44223fee3c70a2 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 26 Apr 2025 19:17:28 -0400 Subject: do_umount(): simplify the "is it still mounted" checks Calls of do_umount() are always preceded by can_umount(), where we'd done a racy check for mount belonging to our namespace; if it wasn't, can_unmount() would've failed with -EINVAL and we wouldn't have reached do_umount() at all. That check needs to be redone once we have acquired namespace_sem and in do_umount() we do that. However, that's done in a very odd way; we check that mount is still in rbtree of _some_ namespace or its mnt_list is not empty. It is equivalent to check_mnt(mnt) - we know that earlier mnt was mounted in our namespace; if it has stayed there, it's going to remain in rbtree of our namespace. OTOH, if it ever had been removed from out namespace, it would be removed from rbtree and it never would've re-added to a namespace afterwards. As for ->mnt_list, for something that had been mounted in a namespace we'll never observe non-empty ->mnt_list while holding namespace_sem - it does temporarily become non-empty during umount_tree(), but that doesn't outlast the call of umount_tree(), let alone dropping namespace_sem. Things get much easier to follow if we replace that with (equivalent) check_mnt(mnt) there. What's more, currently we treat a failure of that test as "quietly do nothing"; we might as well pretend that we'd lost the race and fail on that the same way can_umount() would have. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 12cf69da4320..57b0974a5d1e 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1983,8 +1983,11 @@ static int do_umount(struct mount *mnt, int flags) namespace_lock(); lock_mount_hash(); - /* Recheck MNT_LOCKED with the locks held */ + /* Repeat the earlier racy checks, now that we are holding the locks */ retval = -EINVAL; + if (!check_mnt(mnt)) + goto out; + if (mnt->mnt.mnt_flags & MNT_LOCKED) goto out; @@ -1993,16 +1996,14 @@ static int do_umount(struct mount *mnt, int flags) event++; if (flags & MNT_DETACH) { - if (mnt_ns_attached(mnt) || !list_empty(&mnt->mnt_list)) - umount_tree(mnt, UMOUNT_PROPAGATE); + umount_tree(mnt, UMOUNT_PROPAGATE); retval = 0; } else { smp_mb(); // paired with __legitimize_mnt() shrink_submounts(mnt); retval = -EBUSY; if (!propagate_mount_busy(mnt, 2)) { - if (mnt_ns_attached(mnt) || !list_empty(&mnt->mnt_list)) - umount_tree(mnt, UMOUNT_PROPAGATE|UMOUNT_SYNC); + umount_tree(mnt, UMOUNT_PROPAGATE|UMOUNT_SYNC); retval = 0; } } -- cgit v1.2.3 From 24368a744bafce7daf1eafd6a163871925ee5892 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 2 May 2025 21:32:01 -0400 Subject: sanitize handling of long-term internal mounts Original rationale for those had been the reduced cost of mntput() for the stuff that is mounted somewhere. Mount refcount increments and decrements are frequent; what's worse, they tend to concentrate on the same instances and cacheline pingpong is quite noticable. As the result, mount refcounts are per-cpu; that allows a very cheap increment. Plain decrement would be just as easy, but decrement-and-test is anything but (we need to add the components up, with exclusion against possible increment-from-zero, etc.). Fortunately, there is a very common case where we can tell that decrement won't be the final one - if the thing we are dropping is currently mounted somewhere. We have an RCU delay between the removal from mount tree and dropping the reference that used to pin it there, so we can just take rcu_read_lock() and check if the victim is mounted somewhere. If it is, we can go ahead and decrement without and further checks - the reference we are dropping is not the last one. If it isn't, we get all the fun with locking, carefully adding up components, etc., but the majority of refcount decrements end up taking the fast path. There is a major exception, though - pipes and sockets. Those live on the internal filesystems that are not going to be mounted anywhere. They are not going to be _un_mounted, of course, so having to take the slow path every time a pipe or socket gets closed is really obnoxious. Solution had been to mark them as long-lived ones - essentially faking "they are mounted somewhere" indicator. With minor modification that works even for ones that do eventually get dropped - all it takes is making sure we have an RCU delay between clearing the "mounted somewhere" indicator and dropping the reference. There are some additional twists (if you want to drop a dozen of such internal mounts, you'd be better off with clearing the indicator on all of them, doing an RCU delay once, then dropping the references), but in the basic form it had been * use kern_mount() if you want your internal mount to be a long-term one. * use kern_unmount() to undo that. Unfortunately, the things did rot a bit during the mount API reshuffling. In several cases we have lost the "fake the indicator" part; kern_unmount() on the unmount side remained (it doesn't warn if you use it on a mount without the indicator), but all benefits regaring mntput() cost had been lost. To get rid of that bitrot, let's add a new helper that would work with fs_context-based API: fc_mount_longterm(). It's a counterpart of fc_mount() that does, on success, mark its result as long-term. It must be paired with kern_unmount() or equivalents. Converted: 1) mqueue (it used to use kern_mount_data() and the umount side is still as it used to be) 2) hugetlbfs (used to use kern_mount_data(), internal mount is never unmounted in this one) 3) i915 gemfs (used to be kern_mount() + manual remount to set options, still uses kern_unmount() on umount side) 4) v3d gemfs (copied from i915) Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/hugetlbfs/inode.c | 2 +- fs/namespace.c | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index e4de5425838d..4e0397775167 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -1587,7 +1587,7 @@ static struct vfsmount *__init mount_one_hugetlbfs(struct hstate *h) } else { struct hugetlbfs_fs_context *ctx = fc->fs_private; ctx->hstate = h; - mnt = fc_mount(fc); + mnt = fc_mount_longterm(fc); put_fs_context(fc); } if (IS_ERR(mnt)) diff --git a/fs/namespace.c b/fs/namespace.c index 57b0974a5d1e..6a0697eeda74 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1260,6 +1260,15 @@ struct vfsmount *fc_mount(struct fs_context *fc) } EXPORT_SYMBOL(fc_mount); +struct vfsmount *fc_mount_longterm(struct fs_context *fc) +{ + struct vfsmount *mnt = fc_mount(fc); + if (!IS_ERR(mnt)) + real_mount(mnt)->mnt_ns = MNT_NS_INTERNAL; + return mnt; +} +EXPORT_SYMBOL(fc_mount_longterm); + struct vfsmount *vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void *data) -- cgit v1.2.3 From f0d0ba19985d23a3e83d654318ccb6e9c5f1b095 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 14 May 2025 20:50:06 -0400 Subject: Rewrite of propagate_umount() The variant currently in the tree has problems; trying to prove correctness has caught at least one class of bugs (reparenting that ends up moving the visible location of reparented mount, due to not excluding some of the counterparts on propagation that should've been included). I tried to prove that it's the only bug there; I'm still not sure whether it is. If anyone can reconstruct and write down an analysis of the mainline implementation, I'll gladly review it; as it is, I ended up doing a different implementation. Candidate collection phase is similar, but trimming the set down until it satisfies the constraints turned out pretty different. I hoped to do transformation as a massage series, but that turns out to be too convoluted. So it's a single patch replacing propagate_umount() and friends in one go, with notes and analysis in D/f/propagate_umount.txt (in addition to inline comments). As far I can tell, it is provably correct and provably linear by the number of mounts we need to look at in order to decide what should be unmounted. It even builds and seems to survive testing... Another nice thing that fell out of that is that ->mnt_umounting is no longer needed. Compared to the first version: * explicit MNT_UMOUNT_CANDIDATE flag for is_candidate() * trim_ancestors() only clears that flag, leaving the suckers on list * trim_one() and handle_locked() take the stuff with flag cleared off the list. That allows to iterate with list_for_each_entry_safe() when calling trim_one() - it removes at most one element from the list now. * no globals - I didn't bother with any kind of context, not worth it. * Notes updated accordingly; I have not touch the terms yet. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/mount.h | 1 - fs/namespace.c | 1 - fs/pnode.c | 362 +++++++++++++++++++++++++++++++-------------------------- fs/pnode.h | 2 +- 4 files changed, 199 insertions(+), 167 deletions(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index f20e6ed845fe..fb93d3e16724 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -79,7 +79,6 @@ struct mount { struct hlist_node mnt_mp_list; /* list mounts with the same mountpoint */ struct hlist_node mnt_umount; }; - struct list_head mnt_umounting; /* list entry for umount propagation */ #ifdef CONFIG_FSNOTIFY struct fsnotify_mark_connector __rcu *mnt_fsnotify_marks; __u32 mnt_fsnotify_mask; diff --git a/fs/namespace.c b/fs/namespace.c index 6a0697eeda74..f64895d47d70 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -383,7 +383,6 @@ static struct mount *alloc_vfsmnt(const char *name) INIT_LIST_HEAD(&mnt->mnt_slave_list); INIT_LIST_HEAD(&mnt->mnt_slave); INIT_HLIST_NODE(&mnt->mnt_mp_list); - INIT_LIST_HEAD(&mnt->mnt_umounting); INIT_HLIST_HEAD(&mnt->mnt_stuck_children); RB_CLEAR_NODE(&mnt->mnt_node); mnt->mnt.mnt_idmap = &nop_mnt_idmap; diff --git a/fs/pnode.c b/fs/pnode.c index aa187144e389..901d40946d34 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -24,11 +24,6 @@ static inline struct mount *first_slave(struct mount *p) return list_entry(p->mnt_slave_list.next, struct mount, mnt_slave); } -static inline struct mount *last_slave(struct mount *p) -{ - return list_entry(p->mnt_slave_list.prev, struct mount, mnt_slave); -} - static inline struct mount *next_slave(struct mount *p) { return list_entry(p->mnt_slave.next, struct mount, mnt_slave); @@ -136,6 +131,23 @@ void change_mnt_propagation(struct mount *mnt, int type) } } +static struct mount *__propagation_next(struct mount *m, + struct mount *origin) +{ + while (1) { + struct mount *master = m->mnt_master; + + if (master == origin->mnt_master) { + struct mount *next = next_peer(m); + return (next == origin) ? NULL : next; + } else if (m->mnt_slave.next != &master->mnt_slave_list) + return next_slave(m); + + /* back at master */ + m = master; + } +} + /* * get the next mount in the propagation tree. * @m: the mount seen last @@ -153,31 +165,21 @@ static struct mount *propagation_next(struct mount *m, if (!IS_MNT_NEW(m) && !list_empty(&m->mnt_slave_list)) return first_slave(m); - while (1) { - struct mount *master = m->mnt_master; - - if (master == origin->mnt_master) { - struct mount *next = next_peer(m); - return (next == origin) ? NULL : next; - } else if (m->mnt_slave.next != &master->mnt_slave_list) - return next_slave(m); - - /* back at master */ - m = master; - } + return __propagation_next(m, origin); } static struct mount *skip_propagation_subtree(struct mount *m, struct mount *origin) { /* - * Advance m such that propagation_next will not return - * the slaves of m. + * Advance m past everything that gets propagation from it. */ - if (!IS_MNT_NEW(m) && !list_empty(&m->mnt_slave_list)) - m = last_slave(m); + struct mount *p = __propagation_next(m, origin); + + while (p && peers(m, p)) + p = __propagation_next(p, origin); - return m; + return p; } static struct mount *next_group(struct mount *m, struct mount *origin) @@ -458,181 +460,213 @@ void propagate_mount_unlock(struct mount *mnt) } } -static void umount_one(struct mount *mnt, struct list_head *to_umount) +static inline bool is_candidate(struct mount *m) { - CLEAR_MNT_MARK(mnt); - mnt->mnt.mnt_flags |= MNT_UMOUNT; - list_del_init(&mnt->mnt_child); - list_del_init(&mnt->mnt_umounting); - move_from_ns(mnt, to_umount); + return m->mnt.mnt_flags & MNT_UMOUNT_CANDIDATE; } -/* - * NOTE: unmounting 'mnt' naturally propagates to all other mounts its - * parent propagates to. - */ -static bool __propagate_umount(struct mount *mnt, - struct list_head *to_umount, - struct list_head *to_restore) +static inline bool will_be_unmounted(struct mount *m) { - bool progress = false; - struct mount *child; + return m->mnt.mnt_flags & MNT_UMOUNT; +} - /* - * The state of the parent won't change if this mount is - * already unmounted or marked as without children. - */ - if (mnt->mnt.mnt_flags & (MNT_UMOUNT | MNT_MARKED)) - goto out; +static void umount_one(struct mount *m, struct list_head *to_umount) +{ + m->mnt.mnt_flags |= MNT_UMOUNT; + list_del_init(&m->mnt_child); + move_from_ns(m, to_umount); +} - /* Verify topper is the only grandchild that has not been - * speculatively unmounted. - */ - list_for_each_entry(child, &mnt->mnt_mounts, mnt_child) { - if (child->mnt_mountpoint == mnt->mnt.mnt_root) - continue; - if (!list_empty(&child->mnt_umounting) && IS_MNT_MARKED(child)) - continue; - /* Found a mounted child */ - goto children; - } +static void remove_from_candidate_list(struct mount *m) +{ + m->mnt.mnt_flags &= ~(MNT_MARKED | MNT_UMOUNT_CANDIDATE); + list_del_init(&m->mnt_list); +} - /* Mark mounts that can be unmounted if not locked */ - SET_MNT_MARK(mnt); - progress = true; +static void gather_candidates(struct list_head *set, + struct list_head *candidates) +{ + struct mount *m, *p, *q; - /* If a mount is without children and not locked umount it. */ - if (!IS_MNT_LOCKED(mnt)) { - umount_one(mnt, to_umount); - } else { -children: - list_move_tail(&mnt->mnt_umounting, to_restore); + list_for_each_entry(m, set, mnt_list) { + if (is_candidate(m)) + continue; + m->mnt.mnt_flags |= MNT_UMOUNT_CANDIDATE; + p = m->mnt_parent; + q = propagation_next(p, p); + while (q) { + struct mount *child = __lookup_mnt(&q->mnt, + m->mnt_mountpoint); + if (child) { + /* + * We might've already run into this one. That + * must've happened on earlier iteration of the + * outer loop; in that case we can skip those + * parents that get propagation from q - there + * will be nothing new on those as well. + */ + if (is_candidate(child)) { + q = skip_propagation_subtree(q, p); + continue; + } + child->mnt.mnt_flags |= MNT_UMOUNT_CANDIDATE; + if (!will_be_unmounted(child)) + list_add(&child->mnt_list, candidates); + } + q = propagation_next(q, p); + } } -out: - return progress; + list_for_each_entry(m, set, mnt_list) + m->mnt.mnt_flags &= ~MNT_UMOUNT_CANDIDATE; } -static void umount_list(struct list_head *to_umount, - struct list_head *to_restore) +/* + * We know that some child of @m can't be unmounted. In all places where the + * chain of descent of @m has child not overmounting the root of parent, + * the parent can't be unmounted either. + */ +static void trim_ancestors(struct mount *m) { - struct mount *mnt, *child, *tmp; - list_for_each_entry(mnt, to_umount, mnt_list) { - list_for_each_entry_safe(child, tmp, &mnt->mnt_mounts, mnt_child) { - /* topper? */ - if (child->mnt_mountpoint == mnt->mnt.mnt_root) - list_move_tail(&child->mnt_umounting, to_restore); - else - umount_one(child, to_umount); - } + struct mount *p; + + for (p = m->mnt_parent; is_candidate(p); m = p, p = p->mnt_parent) { + if (IS_MNT_MARKED(m)) // all candidates beneath are overmounts + return; + SET_MNT_MARK(m); + if (m != p->overmount) + p->mnt.mnt_flags &= ~MNT_UMOUNT_CANDIDATE; } } -static void restore_mounts(struct list_head *to_restore) +/* + * Find and exclude all umount candidates forbidden by @m + * (see Documentation/filesystems/propagate_umount.txt) + * If we can immediately tell that @m is OK to unmount (unlocked + * and all children are already committed to unmounting) commit + * to unmounting it. + * Only @m itself might be taken from the candidates list; + * anything found by trim_ancestors() is marked non-candidate + * and left on the list. + */ +static void trim_one(struct mount *m, struct list_head *to_umount) { - /* Restore mounts to a clean working state */ - while (!list_empty(to_restore)) { - struct mount *mnt, *parent; - struct mountpoint *mp; - - mnt = list_first_entry(to_restore, struct mount, mnt_umounting); - CLEAR_MNT_MARK(mnt); - list_del_init(&mnt->mnt_umounting); - - /* Should this mount be reparented? */ - mp = mnt->mnt_mp; - parent = mnt->mnt_parent; - while (parent->mnt.mnt_flags & MNT_UMOUNT) { - mp = parent->mnt_mp; - parent = parent->mnt_parent; - } - if (parent != mnt->mnt_parent) { - mnt_change_mountpoint(parent, mp, mnt); - mnt_notify_add(mnt); + bool remove_this = false, found = false, umount_this = false; + struct mount *n; + + if (!is_candidate(m)) { // trim_ancestors() left it on list + remove_from_candidate_list(m); + return; + } + + list_for_each_entry(n, &m->mnt_mounts, mnt_child) { + if (!is_candidate(n)) { + found = true; + if (n != m->overmount) { + remove_this = true; + break; + } } } + if (found) { + trim_ancestors(m); + } else if (!IS_MNT_LOCKED(m) && list_empty(&m->mnt_mounts)) { + remove_this = true; + umount_this = true; + } + if (remove_this) { + remove_from_candidate_list(m); + if (umount_this) + umount_one(m, to_umount); + } } -static void cleanup_umount_visitations(struct list_head *visited) +static void handle_locked(struct mount *m, struct list_head *to_umount) { - while (!list_empty(visited)) { - struct mount *mnt = - list_first_entry(visited, struct mount, mnt_umounting); - list_del_init(&mnt->mnt_umounting); + struct mount *cutoff = m, *p; + + if (!is_candidate(m)) { // trim_ancestors() left it on list + remove_from_candidate_list(m); + return; + } + for (p = m; is_candidate(p); p = p->mnt_parent) { + remove_from_candidate_list(p); + if (!IS_MNT_LOCKED(p)) + cutoff = p->mnt_parent; + } + if (will_be_unmounted(p)) + cutoff = p; + while (m != cutoff) { + umount_one(m, to_umount); + m = m->mnt_parent; } } /* - * collect all mounts that receive propagation from the mount in @list, - * and return these additional mounts in the same list. - * @list: the list of mounts to be unmounted. + * @m is not to going away, and it overmounts the top of a stack of mounts + * that are going away. We know that all of those are fully overmounted + * by the one above (@m being the topmost of the chain), so @m can be slid + * in place where the bottom of the stack is attached. * - * vfsmount lock must be held for write + * NOTE: here we temporarily violate a constraint - two mounts end up with + * the same parent and mountpoint; that will be remedied as soon as we + * return from propagate_umount() - its caller (umount_tree()) will detach + * the stack from the parent it (and now @m) is attached to. umount_tree() + * might choose to keep unmounted pieces stuck to each other, but it always + * detaches them from the mounts that remain in the tree. */ -int propagate_umount(struct list_head *list) +static void reparent(struct mount *m) { - struct mount *mnt; - LIST_HEAD(to_restore); - LIST_HEAD(to_umount); - LIST_HEAD(visited); - - /* Find candidates for unmounting */ - list_for_each_entry_reverse(mnt, list, mnt_list) { - struct mount *parent = mnt->mnt_parent; - struct mount *m; + struct mount *p = m; + struct mountpoint *mp; - /* - * If this mount has already been visited it is known that it's - * entire peer group and all of their slaves in the propagation - * tree for the mountpoint has already been visited and there is - * no need to visit them again. - */ - if (!list_empty(&mnt->mnt_umounting)) - continue; + do { + mp = p->mnt_mp; + p = p->mnt_parent; + } while (will_be_unmounted(p)); - list_add_tail(&mnt->mnt_umounting, &visited); - for (m = propagation_next(parent, parent); m; - m = propagation_next(m, parent)) { - struct mount *child = __lookup_mnt(&m->mnt, - mnt->mnt_mountpoint); - if (!child) - continue; + mnt_change_mountpoint(p, mp, m); + mnt_notify_add(m); +} - if (!list_empty(&child->mnt_umounting)) { - /* - * If the child has already been visited it is - * know that it's entire peer group and all of - * their slaves in the propgation tree for the - * mountpoint has already been visited and there - * is no need to visit this subtree again. - */ - m = skip_propagation_subtree(m, parent); - continue; - } else if (child->mnt.mnt_flags & MNT_UMOUNT) { - /* - * We have come across a partially unmounted - * mount in a list that has not been visited - * yet. Remember it has been visited and - * continue about our merry way. - */ - list_add_tail(&child->mnt_umounting, &visited); - continue; - } +/** + * propagate_umount - apply propagation rules to the set of mounts for umount() + * @set: the list of mounts to be unmounted. + * + * Collect all mounts that receive propagation from the mount in @set and have + * no obstacles to being unmounted. Add these additional mounts to the set. + * + * See Documentation/filesystems/propagate_umount.txt if you do anything in + * this area. + * + * Locks held: + * mount_lock (write_seqlock), namespace_sem (exclusive). + */ +void propagate_umount(struct list_head *set) +{ + struct mount *m, *p; + LIST_HEAD(to_umount); // committed to unmounting + LIST_HEAD(candidates); // undecided umount candidates - /* Check the child and parents while progress is made */ - while (__propagate_umount(child, - &to_umount, &to_restore)) { - /* Is the parent a umount candidate? */ - child = child->mnt_parent; - if (list_empty(&child->mnt_umounting)) - break; - } - } + // collect all candidates + gather_candidates(set, &candidates); + + // reduce the set until it's non-shifting + list_for_each_entry_safe(m, p, &candidates, mnt_list) + trim_one(m, &to_umount); + + // ... and non-revealing + while (!list_empty(&candidates)) { + m = list_first_entry(&candidates,struct mount, mnt_list); + handle_locked(m, &to_umount); } - umount_list(&to_umount, &to_restore); - restore_mounts(&to_restore); - cleanup_umount_visitations(&visited); - list_splice_tail(&to_umount, list); + // now to_umount consists of all acceptable candidates + // deal with reparenting of remaining overmounts on those + list_for_each_entry(m, &to_umount, mnt_list) { + if (m->overmount) + reparent(m->overmount); + } - return 0; + // and fold them into the set + list_splice_tail_init(&to_umount, set); } diff --git a/fs/pnode.h b/fs/pnode.h index 93fa9311bd07..04f1ac53aa49 100644 --- a/fs/pnode.h +++ b/fs/pnode.h @@ -42,7 +42,7 @@ static inline bool peers(const struct mount *m1, const struct mount *m2) void change_mnt_propagation(struct mount *, int); int propagate_mnt(struct mount *, struct mountpoint *, struct mount *, struct hlist_head *); -int propagate_umount(struct list_head *); +void propagate_umount(struct list_head *); int propagate_mount_busy(struct mount *, int); void propagate_mount_unlock(struct mount *); void mnt_release_group_id(struct mount *); -- cgit v1.2.3 From 7c6fb47b2b6c1ffcb1cae9372b9fcf269444487a Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 22:34:33 -0400 Subject: make commit_tree() usable in same-namespace move case Once attach_recursive_mnt() has created all copies of original subtree, it needs to put them in place(s). Steps needed for those are slightly different: 1) in 'move' case, original copy doesn't need any rbtree manipulations (everything's already in the same namespace where it will be), but it needs to be detached from the current location 2) in 'attach' case, original may be in anon namespace; if it is, all those mounts need to removed from their current namespace before insertion into the target one 3) additional copies have a couple of extra twists - in case of cross-userns propagation we need to lock everything other the root of subtree and in case when we end up inserting under an existing mount, that mount needs to be found (for original copy we have it explicitly passed by the caller). Quite a bit of that can be unified; as the first step, make commit_tree() helper (inserting mounts into namespace, hashing the root of subtree and marking the namespace as updated) usable in all cases; (2) and (3) are already using it and for (1) we only need to make the insertion of mounts into namespace conditional. Signed-off-by: Al Viro --- fs/namespace.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index f64895d47d70..937c2a1825f2 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1172,15 +1172,17 @@ static void commit_tree(struct mount *mnt) BUG_ON(parent == mnt); - list_add_tail(&head, &mnt->mnt_list); - while (!list_empty(&head)) { - m = list_first_entry(&head, typeof(*m), mnt_list); - list_del(&m->mnt_list); + if (!mnt_ns_attached(mnt)) { + list_add_tail(&head, &mnt->mnt_list); + while (!list_empty(&head)) { + m = list_first_entry(&head, typeof(*m), mnt_list); + list_del(&m->mnt_list); - mnt_add_to_ns(n, m); + mnt_add_to_ns(n, m); + } + n->nr_mounts += n->pending_mounts; + n->pending_mounts = 0; } - n->nr_mounts += n->pending_mounts; - n->pending_mounts = 0; make_visible(mnt); touch_mnt_namespace(n); @@ -2691,12 +2693,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, if (moving) { unhash_mnt(source_mnt); - mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); - if (beneath) - mnt_change_mountpoint(top, smp, top_mnt); - make_visible(source_mnt); mnt_notify_add(source_mnt); - touch_mnt_namespace(source_mnt->mnt_ns); } else { if (source_mnt->mnt_ns) { LIST_HEAD(head); @@ -2706,12 +2703,13 @@ static int attach_recursive_mnt(struct mount *source_mnt, move_from_ns(p, &head); list_del_init(&head); } - mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); - if (beneath) - mnt_change_mountpoint(top, smp, top_mnt); - commit_tree(source_mnt); } + mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); + if (beneath) + mnt_change_mountpoint(top, smp, top_mnt); + commit_tree(source_mnt); + hlist_for_each_entry_safe(child, n, &tree_list, mnt_hash) { struct mount *q; hlist_del_init(&child->mnt_hash); -- cgit v1.2.3 From 96f5d2e051653f2cfe9137c5d3c7794c358ffb03 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 22:40:48 -0400 Subject: attach_recursive_mnt(): unify the mnt_change_mountpoint() logics The logics used for tucking under existing mount differs for original and copies; copies do a mount hash lookup to see if mountpoint to be is already overmounted, while the original is told explicitly. But the same logics that is used for copies works for the original, at which point the only place where we get very close to eliminating the need of passing 'beneath' flag to attach_recursive_mnt(). Signed-off-by: Al Viro --- fs/namespace.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 937c2a1825f2..9b8d07df4aa5 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2643,7 +2643,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, HLIST_HEAD(tree_list); struct mnt_namespace *ns = top_mnt->mnt_ns; struct mountpoint *smp; - struct mountpoint *secondary = NULL; + struct mountpoint *shorter = NULL; struct mount *child, *dest_mnt, *p; struct mount *top; struct hlist_node *n; @@ -2655,14 +2655,12 @@ static int attach_recursive_mnt(struct mount *source_mnt, * mounted beneath mounts on the same mountpoint. */ for (top = source_mnt; unlikely(top->overmount); top = top->overmount) { - if (!secondary && is_mnt_ns_file(top->mnt.mnt_root)) - secondary = top->mnt_mp; + if (!shorter && is_mnt_ns_file(top->mnt.mnt_root)) + shorter = top->mnt_mp; } smp = get_mountpoint(top->mnt.mnt_root); if (IS_ERR(smp)) return PTR_ERR(smp); - if (!secondary) - secondary = smp; /* Is there space to add these mounts to the mount namespace? */ if (!moving) { @@ -2706,9 +2704,14 @@ static int attach_recursive_mnt(struct mount *source_mnt, } mnt_set_mountpoint(dest_mnt, dest_mp, source_mnt); - if (beneath) - mnt_change_mountpoint(top, smp, top_mnt); - commit_tree(source_mnt); + /* + * Now the original copy is in the same state as the secondaries - + * its root attached to mountpoint, but not hashed and all mounts + * in it are either in our namespace or in no namespace at all. + * Add the original to the list of copies and deal with the + * rest of work for all of them uniformly. + */ + hlist_add_head(&source_mnt->mnt_hash, &tree_list); hlist_for_each_entry_safe(child, n, &tree_list, mnt_hash) { struct mount *q; @@ -2719,10 +2722,13 @@ static int attach_recursive_mnt(struct mount *source_mnt, q = __lookup_mnt(&child->mnt_parent->mnt, child->mnt_mountpoint); if (q) { + struct mountpoint *mp = smp; struct mount *r = child; while (unlikely(r->overmount)) r = r->overmount; - mnt_change_mountpoint(r, secondary, q); + if (unlikely(shorter) && child != source_mnt) + mp = shorter; + mnt_change_mountpoint(r, mp, q); } commit_tree(child); } -- cgit v1.2.3 From 18959bf585a85c526c88ae23bd223e1afe997bd7 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 22:49:47 -0400 Subject: attach_recursive_mnt(): pass destination mount in all cases ... and 'beneath' is no longer used there Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 9b8d07df4aa5..449e66436b4f 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2570,7 +2570,7 @@ enum mnt_tree_flags_t { /** * attach_recursive_mnt - attach a source mount tree * @source_mnt: mount tree to be attached - * @top_mnt: mount that @source_mnt will be mounted on or mounted beneath + * @dest_mnt: mount that @source_mnt will be mounted on * @dest_mp: the mountpoint @source_mnt will be mounted at * @flags: modify how @source_mnt is supposed to be attached * @@ -2635,20 +2635,20 @@ enum mnt_tree_flags_t { * Otherwise a negative error code is returned. */ static int attach_recursive_mnt(struct mount *source_mnt, - struct mount *top_mnt, + struct mount *dest_mnt, struct mountpoint *dest_mp, enum mnt_tree_flags_t flags) { struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns; HLIST_HEAD(tree_list); - struct mnt_namespace *ns = top_mnt->mnt_ns; + struct mnt_namespace *ns = dest_mnt->mnt_ns; struct mountpoint *smp; struct mountpoint *shorter = NULL; - struct mount *child, *dest_mnt, *p; + struct mount *child, *p; struct mount *top; struct hlist_node *n; int err = 0; - bool moving = flags & MNT_TREE_MOVE, beneath = flags & MNT_TREE_BENEATH; + bool moving = flags & MNT_TREE_MOVE; /* * Preallocate a mountpoint in case the new mounts need to be @@ -2669,11 +2669,6 @@ static int attach_recursive_mnt(struct mount *source_mnt, goto out; } - if (beneath) - dest_mnt = top_mnt->mnt_parent; - else - dest_mnt = top_mnt; - if (IS_MNT_SHARED(dest_mnt)) { err = invent_group_ids(source_mnt, true); if (err) @@ -3688,7 +3683,7 @@ static int do_move_mount(struct path *old_path, if (mount_is_ancestor(old, p)) goto out; - err = attach_recursive_mnt(old, real_mount(new_path->mnt), mp, flags); + err = attach_recursive_mnt(old, p, mp, flags); if (err) goto out; -- cgit v1.2.3 From 86b1da96c5aeb816e4a1b60aa2b3bdcd87e28522 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 22:54:56 -0400 Subject: attach_recursive_mnt(): get rid of flags entirely move vs. attach is trivially detected as mnt_has_parent(source_mnt)... Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 449e66436b4f..adb37f06ba68 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2562,9 +2562,8 @@ int count_mounts(struct mnt_namespace *ns, struct mount *mnt) } enum mnt_tree_flags_t { - MNT_TREE_MOVE = BIT(0), - MNT_TREE_BENEATH = BIT(1), - MNT_TREE_PROPAGATION = BIT(2), + MNT_TREE_BENEATH = BIT(0), + MNT_TREE_PROPAGATION = BIT(1), }; /** @@ -2572,7 +2571,6 @@ enum mnt_tree_flags_t { * @source_mnt: mount tree to be attached * @dest_mnt: mount that @source_mnt will be mounted on * @dest_mp: the mountpoint @source_mnt will be mounted at - * @flags: modify how @source_mnt is supposed to be attached * * NOTE: in the table below explains the semantics when a source mount * of a given type is attached to a destination mount of a given type. @@ -2636,8 +2634,7 @@ enum mnt_tree_flags_t { */ static int attach_recursive_mnt(struct mount *source_mnt, struct mount *dest_mnt, - struct mountpoint *dest_mp, - enum mnt_tree_flags_t flags) + struct mountpoint *dest_mp) { struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns; HLIST_HEAD(tree_list); @@ -2648,7 +2645,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, struct mount *top; struct hlist_node *n; int err = 0; - bool moving = flags & MNT_TREE_MOVE; + bool moving = mnt_has_parent(source_mnt); /* * Preallocate a mountpoint in case the new mounts need to be @@ -2871,7 +2868,7 @@ static int graft_tree(struct mount *mnt, struct mount *p, struct mountpoint *mp) d_is_dir(mnt->mnt.mnt_root)) return -ENOTDIR; - return attach_recursive_mnt(mnt, p, mp, 0); + return attach_recursive_mnt(mnt, p, mp); } /* @@ -3613,8 +3610,6 @@ static int do_move_mount(struct path *old_path, p = real_mount(new_path->mnt); parent = old->mnt_parent; attached = mnt_has_parent(old); - if (attached) - flags |= MNT_TREE_MOVE; old_mp = old->mnt_mp; ns = old->mnt_ns; @@ -3668,7 +3663,6 @@ static int do_move_mount(struct path *old_path, err = -EINVAL; p = p->mnt_parent; - flags |= MNT_TREE_BENEATH; } /* @@ -3683,7 +3677,7 @@ static int do_move_mount(struct path *old_path, if (mount_is_ancestor(old, p)) goto out; - err = attach_recursive_mnt(old, p, mp, flags); + err = attach_recursive_mnt(old, p, mp); if (err) goto out; -- cgit v1.2.3 From 761de25854424aa23fd1d7b2bef5c7d184690926 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 12:55:39 -0400 Subject: do_move_mount(): take dropping the old mountpoint into attach_recursive_mnt() ... and fold it with unhash_mnt() there - there's no need to retain a reference to old_mp beyond that point, since by then all mountpoints we were going to add are either explicitly pinned by get_mountpoint() or have stuff already added to them. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index adb37f06ba68..e5f8fde57c99 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2682,7 +2682,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, } if (moving) { - unhash_mnt(source_mnt); + umount_mnt(source_mnt); mnt_notify_add(source_mnt); } else { if (source_mnt->mnt_ns) { @@ -3598,7 +3598,7 @@ static int do_move_mount(struct path *old_path, struct mount *p; struct mount *old; struct mount *parent; - struct mountpoint *mp, *old_mp; + struct mountpoint *mp; int err; bool attached, beneath = flags & MNT_TREE_BENEATH; @@ -3610,7 +3610,6 @@ static int do_move_mount(struct path *old_path, p = real_mount(new_path->mnt); parent = old->mnt_parent; attached = mnt_has_parent(old); - old_mp = old->mnt_mp; ns = old->mnt_ns; err = -EINVAL; @@ -3684,8 +3683,6 @@ static int do_move_mount(struct path *old_path, /* if the mount is moved, it should no longer be expire * automatically */ list_del_init(&old->mnt_expire); - if (attached) - put_mountpoint(old_mp); out: unlock_mount(mp); if (!err) { -- cgit v1.2.3 From ee1ee33ccc1ba0620a77833b2a3e320588701217 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 8 May 2025 00:09:30 -0400 Subject: do_move_mount(): get rid of 'attached' flag 'attached' serves as a proxy for "source is a subtree of our namespace and not the entirety of anon namespace"; finish massaging it away. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index e5f8fde57c99..7c7cc14da1ee 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -3600,7 +3600,7 @@ static int do_move_mount(struct path *old_path, struct mount *parent; struct mountpoint *mp; int err; - bool attached, beneath = flags & MNT_TREE_BENEATH; + bool beneath = flags & MNT_TREE_BENEATH; mp = do_lock_mount(new_path, beneath); if (IS_ERR(mp)) @@ -3609,7 +3609,6 @@ static int do_move_mount(struct path *old_path, old = real_mount(old_path->mnt); p = real_mount(new_path->mnt); parent = old->mnt_parent; - attached = mnt_has_parent(old); ns = old->mnt_ns; err = -EINVAL; @@ -3622,6 +3621,9 @@ static int do_move_mount(struct path *old_path, /* ... and the target should be in our namespace */ if (!check_mnt(p)) goto out; + /* parent of the source should not be shared */ + if (IS_MNT_SHARED(parent)) + goto out; } else { /* * otherwise the source must be the root of some anon namespace. @@ -3649,11 +3651,6 @@ static int do_move_mount(struct path *old_path, if (d_is_dir(new_path->dentry) != d_is_dir(old_path->dentry)) goto out; - /* - * Don't move a mount residing in a shared parent. - */ - if (attached && IS_MNT_SHARED(parent)) - goto out; if (beneath) { err = can_move_mount_beneath(old_path, new_path, mp); @@ -3686,7 +3683,7 @@ static int do_move_mount(struct path *old_path, out: unlock_mount(mp); if (!err) { - if (attached) { + if (!is_anon_ns(ns)) { mntput_no_expire(parent); } else { /* Make sure we notice when we leak mounts. */ -- cgit v1.2.3 From a8c764e1a580c2128e905ad6e42beef413fb7177 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 1 May 2025 19:59:30 -0400 Subject: attach_recursive_mnt(): remove from expiry list on move ... rather than doing that in do_move_mount(). That's the main obstacle to moving the protection of ->mnt_expire from namespace_sem to mount_lock (spinlock-only), which would simplify several failure exits. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 7c7cc14da1ee..e8dc8af87548 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2684,6 +2684,9 @@ static int attach_recursive_mnt(struct mount *source_mnt, if (moving) { umount_mnt(source_mnt); mnt_notify_add(source_mnt); + /* if the mount is moved, it should no longer be expired + * automatically */ + list_del_init(&source_mnt->mnt_expire); } else { if (source_mnt->mnt_ns) { LIST_HEAD(head); @@ -3674,12 +3677,6 @@ static int do_move_mount(struct path *old_path, goto out; err = attach_recursive_mnt(old, p, mp); - if (err) - goto out; - - /* if the mount is moved, it should no longer be expire - * automatically */ - list_del_init(&old->mnt_expire); out: unlock_mount(mp); if (!err) { -- cgit v1.2.3 From ec3265a245b22f8b8a0b20e04dd1d3f4f4a9ce09 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 1 May 2025 20:40:57 -0400 Subject: take ->mnt_expire handling under mount_lock [read_seqlock_excl] Doesn't take much massage, and we no longer need to make sure that by the time of final mntput() the victim has been removed from the list. Makes life safer for ->d_automount() instances... Rules: * all ->mnt_expire accesses are under mount_lock. * insertion into the list is done by mnt_set_expiry(), and caller (->d_automount() instance) must hold a reference to mount in question. It shouldn't be done more than once for a mount. * if a mount on an expiry list is not yet mounted, it will be ignored by anything that walks that list. * if the final mntput() finds its victim still on an expiry list (in which case it must've never been mounted - umount_tree() would've taken it out), it will remove the victim from the list. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index e8dc8af87548..ff2281f780dc 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1353,13 +1353,6 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, list_add(&mnt->mnt_slave, &old->mnt_slave); mnt->mnt_master = old->mnt_master; } - - /* stick the duplicate mount on the same expiry list - * as the original if that was on one */ - if (flag & CL_EXPIRE) { - if (!list_empty(&old->mnt_expire)) - list_add(&mnt->mnt_expire, &old->mnt_expire); - } return mnt; out_free: @@ -1452,6 +1445,8 @@ static void mntput_no_expire(struct mount *mnt) rcu_read_unlock(); list_del(&mnt->mnt_instance); + if (unlikely(!list_empty(&mnt->mnt_expire))) + list_del(&mnt->mnt_expire); if (unlikely(!list_empty(&mnt->mnt_mounts))) { struct mount *p, *tmp; @@ -2273,6 +2268,13 @@ struct mount *copy_tree(struct mount *src_root, struct dentry *dentry, lock_mount_hash(); if (src_mnt->mnt.mnt_flags & MNT_LOCKED) dst_mnt->mnt.mnt_flags |= MNT_LOCKED; + if (unlikely(flag & CL_EXPIRE)) { + /* stick the duplicate mount on the same expiry + * list as the original if that was on one */ + if (!list_empty(&src_mnt->mnt_expire)) + list_add(&dst_mnt->mnt_expire, + &src_mnt->mnt_expire); + } list_add_tail(&dst_mnt->mnt_list, &res->mnt_list); attach_mnt(dst_mnt, dst_parent, src_parent->mnt_mp); unlock_mount_hash(); @@ -3891,12 +3893,6 @@ discard_locked: namespace_unlock(); inode_unlock(dentry->d_inode); discard: - /* remove m from any expiration list it may be on */ - if (!list_empty(&mnt->mnt_expire)) { - namespace_lock(); - list_del_init(&mnt->mnt_expire); - namespace_unlock(); - } mntput(m); return err; } @@ -3908,11 +3904,9 @@ discard: */ void mnt_set_expiry(struct vfsmount *mnt, struct list_head *expiry_list) { - namespace_lock(); - + read_seqlock_excl(&mount_lock); list_add_tail(&real_mount(mnt)->mnt_expire, expiry_list); - - namespace_unlock(); + read_sequnlock_excl(&mount_lock); } EXPORT_SYMBOL(mnt_set_expiry); -- cgit v1.2.3 From e30da2a20e31ab958d41e5a2c764968d95f17b61 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 16:53:01 -0400 Subject: pivot_root(): reorder tree surgeries, collapse unhash_mnt() and put_mountpoint() attach new_mnt *before* detaching root_mnt; that way we don't need to keep hold on the mountpoint and one more pair of unhash_mnt()/put_mountpoint() gets folded together into umount_mnt(). Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index ff2281f780dc..eee73e945a54 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -4685,7 +4685,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, { struct path new, old, root; struct mount *new_mnt, *root_mnt, *old_mnt, *root_parent, *ex_parent; - struct mountpoint *old_mp, *root_mp; + struct mountpoint *old_mp; int error; if (!may_mount()) @@ -4748,20 +4748,19 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, goto out4; lock_mount_hash(); umount_mnt(new_mnt); - root_mp = unhash_mnt(root_mnt); /* we'll need its mountpoint */ if (root_mnt->mnt.mnt_flags & MNT_LOCKED) { new_mnt->mnt.mnt_flags |= MNT_LOCKED; root_mnt->mnt.mnt_flags &= ~MNT_LOCKED; } - /* mount old root on put_old */ - attach_mnt(root_mnt, old_mnt, old_mp); /* mount new_root on / */ - attach_mnt(new_mnt, root_parent, root_mp); + attach_mnt(new_mnt, root_parent, root_mnt->mnt_mp); + umount_mnt(root_mnt); mnt_add_count(root_parent, -1); + /* mount old root on put_old */ + attach_mnt(root_mnt, old_mnt, old_mp); touch_mnt_namespace(current->nsproxy->mnt_ns); /* A moved mount should not expire automatically */ list_del_init(&new_mnt->mnt_expire); - put_mountpoint(root_mp); unlock_mount_hash(); mnt_notify_add(root_mnt); mnt_notify_add(new_mnt); -- cgit v1.2.3 From 86f63980964b334ad49d5c1f132f3b9491303a15 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 17:24:10 -0400 Subject: combine __put_mountpoint() with unhash_mnt() A call of unhash_mnt() is immediately followed by passing its return value to __put_mountpoint(); the shrink list given to __put_mountpoint() will be ex_mountpoints when called from umount_mnt() and list when called from mntput_no_expire(). Replace with __umount_mnt(mount, shrink_list), moving the call of __put_mountpoint() into it (and returning nothing), adjust the callers. Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index eee73e945a54..521ffa52c906 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1037,9 +1037,9 @@ static void __touch_mnt_namespace(struct mnt_namespace *ns) } /* - * vfsmount lock must be held for write + * locks: mount_lock[write_seqlock] */ -static struct mountpoint *unhash_mnt(struct mount *mnt) +static void __umount_mnt(struct mount *mnt, struct list_head *shrink_list) { struct mountpoint *mp; struct mount *parent = mnt->mnt_parent; @@ -1052,15 +1052,15 @@ static struct mountpoint *unhash_mnt(struct mount *mnt) hlist_del_init(&mnt->mnt_mp_list); mp = mnt->mnt_mp; mnt->mnt_mp = NULL; - return mp; + __put_mountpoint(mp, shrink_list); } /* - * vfsmount lock must be held for write + * locks: mount_lock[write_seqlock], namespace_sem[excl] (for ex_mountpoints) */ static void umount_mnt(struct mount *mnt) { - put_mountpoint(unhash_mnt(mnt)); + __umount_mnt(mnt, &ex_mountpoints); } /* @@ -1451,7 +1451,7 @@ static void mntput_no_expire(struct mount *mnt) if (unlikely(!list_empty(&mnt->mnt_mounts))) { struct mount *p, *tmp; list_for_each_entry_safe(p, tmp, &mnt->mnt_mounts, mnt_child) { - __put_mountpoint(unhash_mnt(p), &list); + __umount_mnt(p, &list); hlist_add_head(&p->mnt_umount, &mnt->mnt_stuck_children); } } -- cgit v1.2.3 From d72c773237c0472e214cda92016ad21625b05bba Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Apr 2025 20:21:23 -0400 Subject: get rid of mountpoint->m_count struct mountpoint has an odd kinda-sorta refcount in it. It's always either equal to or one above the number of mounts attached to that mountpoint. "One above" happens when a function takes a temporary reference to mountpoint. Things get simpler if we express that as inserting a local object into ->m_list and removing it to drop the reference. New calling conventions: 1) lock_mount(), do_lock_mount(), get_mountpoint() and lookup_mountpoint() take an extra struct pinned_mountpoint * argument and returns 0/-E... (or true/false in case of lookup_mountpoint()) instead of returning struct mountpoint pointers. In case of success, the struct mountpoint * we used to get can be found as pinned_mountpoint.mp 2) unlock_mount() (always paired with lock_mount()/do_lock_mount()) takes an address of struct pinned_mountpoint - the same that had been passed to lock_mount()/do_lock_mount(). 3) put_mountpoint() for a temporary reference (paired with get_mountpoint() or lookup_mountpoint()) is replaced with unpin_mountpoint(), which takes the address of pinned_mountpoint we passed to matching {get,lookup}_mountpoint(). 4) all instances of pinned_mountpoint are local variables; they always live on stack. {} is used for initializer, after successful {get,lookup}_mountpoint() we must make sure to call unpin_mountpoint() before leaving the scope and after successful {do_,}lock_mount() we must make sure to call unlock_mount() before leaving the scope. 5) all manipulations of ->m_count are gone, along with ->m_count itself. struct mountpoint lives while its ->m_list is non-empty. Signed-off-by: Al Viro --- fs/mount.h | 1 - fs/namespace.c | 186 ++++++++++++++++++++++++++++----------------------------- 2 files changed, 92 insertions(+), 95 deletions(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index fb93d3e16724..4355c482a841 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -44,7 +44,6 @@ struct mountpoint { struct hlist_node m_hash; struct dentry *m_dentry; struct hlist_head m_list; - int m_count; }; struct mount { diff --git a/fs/namespace.c b/fs/namespace.c index 521ffa52c906..6df0436bfcb9 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -910,42 +910,48 @@ bool __is_local_mountpoint(const struct dentry *dentry) return is_covered; } -static struct mountpoint *lookup_mountpoint(struct dentry *dentry) +struct pinned_mountpoint { + struct hlist_node node; + struct mountpoint *mp; +}; + +static bool lookup_mountpoint(struct dentry *dentry, struct pinned_mountpoint *m) { struct hlist_head *chain = mp_hash(dentry); struct mountpoint *mp; hlist_for_each_entry(mp, chain, m_hash) { if (mp->m_dentry == dentry) { - mp->m_count++; - return mp; + hlist_add_head(&m->node, &mp->m_list); + m->mp = mp; + return true; } } - return NULL; + return false; } -static struct mountpoint *get_mountpoint(struct dentry *dentry) +static int get_mountpoint(struct dentry *dentry, struct pinned_mountpoint *m) { - struct mountpoint *mp, *new = NULL; + struct mountpoint *mp __free(kfree) = NULL; + bool found; int ret; if (d_mountpoint(dentry)) { /* might be worth a WARN_ON() */ if (d_unlinked(dentry)) - return ERR_PTR(-ENOENT); + return -ENOENT; mountpoint: read_seqlock_excl(&mount_lock); - mp = lookup_mountpoint(dentry); + found = lookup_mountpoint(dentry, m); read_sequnlock_excl(&mount_lock); - if (mp) - goto done; + if (found) + return 0; } - if (!new) - new = kmalloc(sizeof(struct mountpoint), GFP_KERNEL); - if (!new) - return ERR_PTR(-ENOMEM); - + if (!mp) + mp = kmalloc(sizeof(struct mountpoint), GFP_KERNEL); + if (!mp) + return -ENOMEM; /* Exactly one processes may set d_mounted */ ret = d_set_mounted(dentry); @@ -955,34 +961,28 @@ mountpoint: goto mountpoint; /* The dentry is not available as a mountpoint? */ - mp = ERR_PTR(ret); if (ret) - goto done; + return ret; /* Add the new mountpoint to the hash table */ read_seqlock_excl(&mount_lock); - new->m_dentry = dget(dentry); - new->m_count = 1; - hlist_add_head(&new->m_hash, mp_hash(dentry)); - INIT_HLIST_HEAD(&new->m_list); + mp->m_dentry = dget(dentry); + hlist_add_head(&mp->m_hash, mp_hash(dentry)); + INIT_HLIST_HEAD(&mp->m_list); + hlist_add_head(&m->node, &mp->m_list); + m->mp = no_free_ptr(mp); read_sequnlock_excl(&mount_lock); - - mp = new; - new = NULL; -done: - kfree(new); - return mp; + return 0; } /* * vfsmount lock must be held. Additionally, the caller is responsible * for serializing calls for given disposal list. */ -static void __put_mountpoint(struct mountpoint *mp, struct list_head *list) +static void maybe_free_mountpoint(struct mountpoint *mp, struct list_head *list) { - if (!--mp->m_count) { + if (hlist_empty(&mp->m_list)) { struct dentry *dentry = mp->m_dentry; - BUG_ON(!hlist_empty(&mp->m_list)); spin_lock(&dentry->d_lock); dentry->d_flags &= ~DCACHE_MOUNTED; spin_unlock(&dentry->d_lock); @@ -992,10 +992,15 @@ static void __put_mountpoint(struct mountpoint *mp, struct list_head *list) } } -/* called with namespace_lock and vfsmount lock */ -static void put_mountpoint(struct mountpoint *mp) +/* + * locks: mount_lock [read_seqlock_excl], namespace_sem [excl] + */ +static void unpin_mountpoint(struct pinned_mountpoint *m) { - __put_mountpoint(mp, &ex_mountpoints); + if (m->mp) { + hlist_del(&m->node); + maybe_free_mountpoint(m->mp, &ex_mountpoints); + } } static inline int check_mnt(struct mount *mnt) @@ -1052,7 +1057,7 @@ static void __umount_mnt(struct mount *mnt, struct list_head *shrink_list) hlist_del_init(&mnt->mnt_mp_list); mp = mnt->mnt_mp; mnt->mnt_mp = NULL; - __put_mountpoint(mp, shrink_list); + maybe_free_mountpoint(mp, shrink_list); } /* @@ -1070,7 +1075,6 @@ void mnt_set_mountpoint(struct mount *mnt, struct mountpoint *mp, struct mount *child_mnt) { - mp->m_count++; mnt_add_count(mnt, 1); /* essentially, that's mntget */ child_mnt->mnt_mountpoint = mp->m_dentry; child_mnt->mnt_parent = mnt; @@ -1122,7 +1126,7 @@ void mnt_change_mountpoint(struct mount *parent, struct mountpoint *mp, struct m attach_mnt(mnt, parent, mp); - put_mountpoint(old_mp); + maybe_free_mountpoint(old_mp, &ex_mountpoints); mnt_add_count(old_parent, -1); } @@ -2030,25 +2034,24 @@ out: */ void __detach_mounts(struct dentry *dentry) { - struct mountpoint *mp; + struct pinned_mountpoint mp = {}; struct mount *mnt; namespace_lock(); lock_mount_hash(); - mp = lookup_mountpoint(dentry); - if (!mp) + if (!lookup_mountpoint(dentry, &mp)) goto out_unlock; event++; - while (!hlist_empty(&mp->m_list)) { - mnt = hlist_entry(mp->m_list.first, struct mount, mnt_mp_list); + while (mp.node.next) { + mnt = hlist_entry(mp.node.next, struct mount, mnt_mp_list); if (mnt->mnt.mnt_flags & MNT_UMOUNT) { umount_mnt(mnt); hlist_add_head(&mnt->mnt_umount, &unmounted); } else umount_tree(mnt, UMOUNT_CONNECTED); } - put_mountpoint(mp); + unpin_mountpoint(&mp); out_unlock: unlock_mount_hash(); namespace_unlock(); @@ -2641,7 +2644,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns; HLIST_HEAD(tree_list); struct mnt_namespace *ns = dest_mnt->mnt_ns; - struct mountpoint *smp; + struct pinned_mountpoint root = {}; struct mountpoint *shorter = NULL; struct mount *child, *p; struct mount *top; @@ -2657,9 +2660,9 @@ static int attach_recursive_mnt(struct mount *source_mnt, if (!shorter && is_mnt_ns_file(top->mnt.mnt_root)) shorter = top->mnt_mp; } - smp = get_mountpoint(top->mnt.mnt_root); - if (IS_ERR(smp)) - return PTR_ERR(smp); + err = get_mountpoint(top->mnt.mnt_root, &root); + if (err) + return err; /* Is there space to add these mounts to the mount namespace? */ if (!moving) { @@ -2719,7 +2722,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, q = __lookup_mnt(&child->mnt_parent->mnt, child->mnt_mountpoint); if (q) { - struct mountpoint *mp = smp; + struct mountpoint *mp = root.mp; struct mount *r = child; while (unlikely(r->overmount)) r = r->overmount; @@ -2729,7 +2732,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, } commit_tree(child); } - put_mountpoint(smp); + unpin_mountpoint(&root); unlock_mount_hash(); return 0; @@ -2746,7 +2749,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, ns->pending_mounts = 0; read_seqlock_excl(&mount_lock); - put_mountpoint(smp); + unpin_mountpoint(&root); read_sequnlock_excl(&mount_lock); return err; @@ -2786,12 +2789,12 @@ static int attach_recursive_mnt(struct mount *source_mnt, * Return: Either the target mountpoint on the top mount or the top * mount's mountpoint. */ -static struct mountpoint *do_lock_mount(struct path *path, bool beneath) +static int do_lock_mount(struct path *path, struct pinned_mountpoint *pinned, bool beneath) { struct vfsmount *mnt = path->mnt; struct dentry *dentry; - struct mountpoint *mp = ERR_PTR(-ENOENT); struct path under = {}; + int err = -ENOENT; for (;;) { struct mount *m = real_mount(mnt); @@ -2829,8 +2832,8 @@ static struct mountpoint *do_lock_mount(struct path *path, bool beneath) path->dentry = dget(mnt->mnt_root); continue; // got overmounted } - mp = get_mountpoint(dentry); - if (IS_ERR(mp)) + err = get_mountpoint(dentry, pinned); + if (err) break; if (beneath) { /* @@ -2841,25 +2844,25 @@ static struct mountpoint *do_lock_mount(struct path *path, bool beneath) */ path_put(&under); } - return mp; + return 0; } namespace_unlock(); inode_unlock(dentry->d_inode); if (beneath) path_put(&under); - return mp; + return err; } -static inline struct mountpoint *lock_mount(struct path *path) +static inline int lock_mount(struct path *path, struct pinned_mountpoint *m) { - return do_lock_mount(path, false); + return do_lock_mount(path, m, false); } -static void unlock_mount(struct mountpoint *where) +static void unlock_mount(struct pinned_mountpoint *m) { - inode_unlock(where->m_dentry->d_inode); + inode_unlock(m->mp->m_dentry->d_inode); read_seqlock_excl(&mount_lock); - put_mountpoint(where); + unpin_mountpoint(m); read_sequnlock_excl(&mount_lock); namespace_unlock(); } @@ -3024,7 +3027,7 @@ static int do_loopback(struct path *path, const char *old_name, { struct path old_path; struct mount *mnt = NULL, *parent; - struct mountpoint *mp; + struct pinned_mountpoint mp = {}; int err; if (!old_name || !*old_name) return -EINVAL; @@ -3036,11 +3039,9 @@ static int do_loopback(struct path *path, const char *old_name, if (mnt_ns_loop(old_path.dentry)) goto out; - mp = lock_mount(path); - if (IS_ERR(mp)) { - err = PTR_ERR(mp); + err = lock_mount(path, &mp); + if (err) goto out; - } parent = real_mount(path->mnt); if (!check_mnt(parent)) @@ -3052,14 +3053,14 @@ static int do_loopback(struct path *path, const char *old_name, goto out2; } - err = graft_tree(mnt, parent, mp); + err = graft_tree(mnt, parent, mp.mp); if (err) { lock_mount_hash(); umount_tree(mnt, UMOUNT_SYNC); unlock_mount_hash(); } out2: - unlock_mount(mp); + unlock_mount(&mp); out: path_put(&old_path); return err; @@ -3603,13 +3604,13 @@ static int do_move_mount(struct path *old_path, struct mount *p; struct mount *old; struct mount *parent; - struct mountpoint *mp; + struct pinned_mountpoint mp; int err; bool beneath = flags & MNT_TREE_BENEATH; - mp = do_lock_mount(new_path, beneath); - if (IS_ERR(mp)) - return PTR_ERR(mp); + err = do_lock_mount(new_path, &mp, beneath); + if (err) + return err; old = real_mount(old_path->mnt); p = real_mount(new_path->mnt); @@ -3658,7 +3659,7 @@ static int do_move_mount(struct path *old_path, goto out; if (beneath) { - err = can_move_mount_beneath(old_path, new_path, mp); + err = can_move_mount_beneath(old_path, new_path, mp.mp); if (err) goto out; @@ -3678,9 +3679,9 @@ static int do_move_mount(struct path *old_path, if (mount_is_ancestor(old, p)) goto out; - err = attach_recursive_mnt(old, p, mp); + err = attach_recursive_mnt(old, p, mp.mp); out: - unlock_mount(mp); + unlock_mount(&mp); if (!err) { if (!is_anon_ns(ns)) { mntput_no_expire(parent); @@ -3750,7 +3751,7 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint, unsigned int mnt_flags) { struct vfsmount *mnt; - struct mountpoint *mp; + struct pinned_mountpoint mp = {}; struct super_block *sb = fc->root->d_sb; int error; @@ -3771,13 +3772,12 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint, mnt_warn_timestamp_expiry(mountpoint, mnt); - mp = lock_mount(mountpoint); - if (IS_ERR(mp)) { - mntput(mnt); - return PTR_ERR(mp); + error = lock_mount(mountpoint, &mp); + if (!error) { + error = do_add_mount(real_mount(mnt), mp.mp, + mountpoint, mnt_flags); + unlock_mount(&mp); } - error = do_add_mount(real_mount(mnt), mp, mountpoint, mnt_flags); - unlock_mount(mp); if (error < 0) mntput(mnt); return error; @@ -3845,7 +3845,7 @@ static int do_new_mount(struct path *path, const char *fstype, int sb_flags, int finish_automount(struct vfsmount *m, const struct path *path) { struct dentry *dentry = path->dentry; - struct mountpoint *mp; + struct pinned_mountpoint mp = {}; struct mount *mnt; int err; @@ -3877,14 +3877,13 @@ int finish_automount(struct vfsmount *m, const struct path *path) err = 0; goto discard_locked; } - mp = get_mountpoint(dentry); - if (IS_ERR(mp)) { - err = PTR_ERR(mp); + err = get_mountpoint(dentry, &mp); + if (err) goto discard_locked; - } - err = do_add_mount(mnt, mp, path, path->mnt->mnt_flags | MNT_SHRINKABLE); - unlock_mount(mp); + err = do_add_mount(mnt, mp.mp, path, + path->mnt->mnt_flags | MNT_SHRINKABLE); + unlock_mount(&mp); if (unlikely(err)) goto discard; return 0; @@ -4685,7 +4684,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, { struct path new, old, root; struct mount *new_mnt, *root_mnt, *old_mnt, *root_parent, *ex_parent; - struct mountpoint *old_mp; + struct pinned_mountpoint old_mp = {}; int error; if (!may_mount()) @@ -4706,9 +4705,8 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, goto out2; get_fs_root(current->fs, &root); - old_mp = lock_mount(&old); - error = PTR_ERR(old_mp); - if (IS_ERR(old_mp)) + error = lock_mount(&old, &old_mp); + if (error) goto out3; error = -EINVAL; @@ -4757,7 +4755,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, umount_mnt(root_mnt); mnt_add_count(root_parent, -1); /* mount old root on put_old */ - attach_mnt(root_mnt, old_mnt, old_mp); + attach_mnt(root_mnt, old_mnt, old_mp.mp); touch_mnt_namespace(current->nsproxy->mnt_ns); /* A moved mount should not expire automatically */ list_del_init(&new_mnt->mnt_expire); @@ -4767,7 +4765,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, chroot_fs_refs(&root, &new); error = 0; out4: - unlock_mount(old_mp); + unlock_mount(&old_mp); if (!error) mntput_no_expire(ex_parent); out3: -- cgit v1.2.3 From 493a4bebf5157a5da64e36f8d468ff80a859b563 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 27 Apr 2025 22:53:13 -0400 Subject: don't have mounts pin their parents Simplify the rules for mount refcounts. Current rules include: * being a namespace root => +1 * being someone's child => +1 * being someone's child => +1 to parent's refcount, unless you've already been through umount_tree(). The last part is not needed at all. It makes for more places where need to decrement refcounts and it creates an asymmetry between the situations for something that has never been a part of a namespace and something that left one, both for no good reason. If mount's refcount has additions from its children, we know that * it's either someone's child itself (and will remain so until umount_tree(), at which point contributions from children will disappear), or * or is the root of namespace (and will remain such until it either becomes someone's child in another namespace or goes through umount_tree()), or * it is the root of some tree copy, and is currently pinned by the caller of copy_tree() (and remains such until it either gets into namespace, or goes to umount_tree()). In all cases we already have contribution(s) to refcount that will last as long as the contribution from children remains. In other words, the lifetime is not affected by refcount contributions from children. It might be useful for "is it busy" checks, but those are actually no harder to express without it. NB: propagate_mnt_busy() part is an equivalent transformation, ugly as it is; the current logics is actually wrong and may give false negatives, but fixing that is for a separate patch (probably earlier in the queue). Reviewed-by: Christian Brauner Signed-off-by: Al Viro --- fs/namespace.c | 31 +++++++++---------------------- fs/pnode.c | 49 +++++++++++++++++-------------------------------- 2 files changed, 26 insertions(+), 54 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 6df0436bfcb9..4bdf6a6e75ca 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1075,7 +1075,6 @@ void mnt_set_mountpoint(struct mount *mnt, struct mountpoint *mp, struct mount *child_mnt) { - mnt_add_count(mnt, 1); /* essentially, that's mntget */ child_mnt->mnt_mountpoint = mp->m_dentry; child_mnt->mnt_parent = mnt; child_mnt->mnt_mp = mp; @@ -1118,7 +1117,6 @@ static void attach_mnt(struct mount *mnt, struct mount *parent, void mnt_change_mountpoint(struct mount *parent, struct mountpoint *mp, struct mount *mnt) { struct mountpoint *old_mp = mnt->mnt_mp; - struct mount *old_parent = mnt->mnt_parent; list_del_init(&mnt->mnt_child); hlist_del_init(&mnt->mnt_mp_list); @@ -1127,7 +1125,6 @@ void mnt_change_mountpoint(struct mount *parent, struct mountpoint *mp, struct m attach_mnt(mnt, parent, mp); maybe_free_mountpoint(old_mp, &ex_mountpoints); - mnt_add_count(old_parent, -1); } static inline struct mount *node_to_mount(struct rb_node *node) @@ -1652,23 +1649,19 @@ const struct seq_operations mounts_op = { int may_umount_tree(struct vfsmount *m) { struct mount *mnt = real_mount(m); - int actual_refs = 0; - int minimum_refs = 0; - struct mount *p; - BUG_ON(!m); + bool busy = false; /* write lock needed for mnt_get_count */ lock_mount_hash(); - for (p = mnt; p; p = next_mnt(p, mnt)) { - actual_refs += mnt_get_count(p); - minimum_refs += 2; + for (struct mount *p = mnt; p; p = next_mnt(p, mnt)) { + if (mnt_get_count(p) > (p == mnt ? 2 : 1)) { + busy = true; + break; + } } unlock_mount_hash(); - if (actual_refs > minimum_refs) - return 0; - - return 1; + return !busy; } EXPORT_SYMBOL(may_umount_tree); @@ -1869,7 +1862,6 @@ static void umount_tree(struct mount *mnt, enum umount_tree_flags how) disconnect = disconnect_mount(p, how); if (mnt_has_parent(p)) { - mnt_add_count(p->mnt_parent, -1); if (!disconnect) { /* Don't forget about p */ list_add_tail(&p->mnt_child, &p->mnt_parent->mnt_mounts); @@ -1946,7 +1938,7 @@ static int do_umount(struct mount *mnt, int flags) * all race cases, but it's a slowpath. */ lock_mount_hash(); - if (mnt_get_count(mnt) != 2) { + if (!list_empty(&mnt->mnt_mounts) || mnt_get_count(mnt) != 2) { unlock_mount_hash(); return -EBUSY; } @@ -3683,9 +3675,7 @@ static int do_move_mount(struct path *old_path, out: unlock_mount(&mp); if (!err) { - if (!is_anon_ns(ns)) { - mntput_no_expire(parent); - } else { + if (is_anon_ns(ns)) { /* Make sure we notice when we leak mounts. */ VFS_WARN_ON_ONCE(!mnt_ns_empty(ns)); free_mnt_ns(ns); @@ -4753,7 +4743,6 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, /* mount new_root on / */ attach_mnt(new_mnt, root_parent, root_mnt->mnt_mp); umount_mnt(root_mnt); - mnt_add_count(root_parent, -1); /* mount old root on put_old */ attach_mnt(root_mnt, old_mnt, old_mp.mp); touch_mnt_namespace(current->nsproxy->mnt_ns); @@ -4766,8 +4755,6 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, error = 0; out4: unlock_mount(&old_mp); - if (!error) - mntput_no_expire(ex_parent); out3: path_put(&root); out2: diff --git a/fs/pnode.c b/fs/pnode.c index 901d40946d34..827d71736ac5 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -332,21 +332,6 @@ out: return ret; } -static struct mount *find_topper(struct mount *mnt) -{ - /* If there is exactly one mount covering mnt completely return it. */ - struct mount *child; - - if (!list_is_singular(&mnt->mnt_mounts)) - return NULL; - - child = list_first_entry(&mnt->mnt_mounts, struct mount, mnt_child); - if (child->mnt_mountpoint != mnt->mnt.mnt_root) - return NULL; - - return child; -} - /* * return true if the refcount is greater than count */ @@ -404,12 +389,8 @@ bool propagation_would_overmount(const struct mount *from, */ int propagate_mount_busy(struct mount *mnt, int refcnt) { - struct mount *m, *child, *topper; struct mount *parent = mnt->mnt_parent; - if (mnt == parent) - return do_refcount_check(mnt, refcnt); - /* * quickly check if the current mount can be unmounted. * If not, we don't have to go checking for all other @@ -418,23 +399,27 @@ int propagate_mount_busy(struct mount *mnt, int refcnt) if (!list_empty(&mnt->mnt_mounts) || do_refcount_check(mnt, refcnt)) return 1; - for (m = propagation_next(parent, parent); m; + if (mnt == parent) + return 0; + + for (struct mount *m = propagation_next(parent, parent); m; m = propagation_next(m, parent)) { - int count = 1; - child = __lookup_mnt(&m->mnt, mnt->mnt_mountpoint); - if (!child) - continue; + struct list_head *head; + struct mount *child = __lookup_mnt(&m->mnt, mnt->mnt_mountpoint); - /* Is there exactly one mount on the child that covers - * it completely whose reference should be ignored? - */ - topper = find_topper(child); - if (topper) - count += 1; - else if (!list_empty(&child->mnt_mounts)) + if (!child) continue; - if (do_refcount_check(child, count)) + 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)) return 1; } return 0; -- cgit v1.2.3 From 406fea79992561f47fd3511dd8b7c8abeeff7045 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 21 Jun 2025 18:06:19 -0400 Subject: mount: separate the flags accessed only under namespace_sem Several flags are updated and checked only under namespace_sem; we are already making use of that when we are checking them without mount_lock, but we have to hold mount_lock for all updates, which makes things clumsier than they have to be. Take MNT_SHARED, MNT_UNBINDABLE, MNT_MARKED and MNT_UMOUNT_CANDIDATE into a separate field (->mnt_t_flags), renaming them to T_SHARED, etc. to avoid confusion. All accesses must be under namespace_sem. That changes locking requirements for mnt_change_propagation() and set_mnt_shared() - only namespace_sem is needed now. The same goes for SET_MNT_MARKED et.al. There might be more flags moved from ->mnt_flags to that field; this is just the initial set. Signed-off-by: Al Viro --- fs/mount.h | 17 +++++++++++++++++ fs/namespace.c | 4 ---- fs/pnode.c | 22 ++++++++++------------ fs/pnode.h | 19 +++++++++++-------- 4 files changed, 38 insertions(+), 24 deletions(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index 4355c482a841..f299dc85446d 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -84,6 +84,7 @@ struct mount { struct list_head to_notify; /* need to queue notification */ struct mnt_namespace *prev_ns; /* previous namespace (NULL if none) */ #endif + int mnt_t_flags; /* namespace_sem-protected flags */ int mnt_id; /* mount identifier, reused */ u64 mnt_id_unique; /* mount ID unique until reboot */ int mnt_group_id; /* peer group identifier */ @@ -93,6 +94,22 @@ struct mount { struct mount *overmount; /* mounted on ->mnt_root */ } __randomize_layout; +enum { + T_SHARED = 1, /* mount is shared */ + T_UNBINDABLE = 2, /* mount is unbindable */ + T_MARKED = 4, /* internal mark for propagate_... */ + T_UMOUNT_CANDIDATE = 8, /* for propagate_umount */ + + /* + * T_SHARED_MASK is the set of flags that should be cleared when a + * mount becomes shared. Currently, this is only the flag that says a + * mount cannot be bind mounted, since this is how we create a mount + * that shares events with another mount. If you add a new T_* + * flag, consider how it interacts with shared mounts. + */ + T_SHARED_MASK = T_UNBINDABLE, +}; + #define MNT_NS_INTERNAL ERR_PTR(-EINVAL) /* distinct from any mnt_namespace */ static inline struct mount *real_mount(struct vfsmount *mnt) diff --git a/fs/namespace.c b/fs/namespace.c index 4bdf6a6e75ca..da27365418a5 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2917,10 +2917,8 @@ static int do_change_type(struct path *path, int ms_flags) goto out_unlock; } - lock_mount_hash(); for (m = mnt; m; m = (recurse ? next_mnt(m, mnt) : NULL)) change_mnt_propagation(m, type); - unlock_mount_hash(); out_unlock: namespace_unlock(); @@ -3409,9 +3407,7 @@ static int do_set_group(struct path *from_path, struct path *to_path) if (IS_MNT_SHARED(from)) { to->mnt_group_id = from->mnt_group_id; list_add(&to->mnt_share, &from->mnt_share); - lock_mount_hash(); set_mnt_shared(to); - unlock_mount_hash(); } err = 0; diff --git a/fs/pnode.c b/fs/pnode.c index 827d71736ac5..b997663de6d0 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -112,7 +112,7 @@ static int do_make_slave(struct mount *mnt) } /* - * vfsmount lock must be held for write + * EXCL[namespace_sem] */ void change_mnt_propagation(struct mount *mnt, int type) { @@ -125,9 +125,9 @@ void change_mnt_propagation(struct mount *mnt, int type) list_del_init(&mnt->mnt_slave); mnt->mnt_master = NULL; if (type == MS_UNBINDABLE) - mnt->mnt.mnt_flags |= MNT_UNBINDABLE; + mnt->mnt_t_flags |= T_UNBINDABLE; else - mnt->mnt.mnt_flags &= ~MNT_UNBINDABLE; + mnt->mnt_t_flags &= ~T_UNBINDABLE; } } @@ -263,9 +263,9 @@ static int propagate_one(struct mount *m, struct mountpoint *dest_mp) return PTR_ERR(child); read_seqlock_excl(&mount_lock); mnt_set_mountpoint(m, dest_mp, child); + read_sequnlock_excl(&mount_lock); if (m->mnt_master != dest_master) SET_MNT_MARK(m->mnt_master); - read_sequnlock_excl(&mount_lock); last_dest = m; last_source = child; hlist_add_head(&child->mnt_hash, list); @@ -322,13 +322,11 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, } while (n != m); } out: - read_seqlock_excl(&mount_lock); hlist_for_each_entry(n, tree_list, mnt_hash) { m = n->mnt_parent; if (m->mnt_master != dest_mnt->mnt_master) CLEAR_MNT_MARK(m->mnt_master); } - read_sequnlock_excl(&mount_lock); return ret; } @@ -447,7 +445,7 @@ void propagate_mount_unlock(struct mount *mnt) static inline bool is_candidate(struct mount *m) { - return m->mnt.mnt_flags & MNT_UMOUNT_CANDIDATE; + return m->mnt_t_flags & T_UMOUNT_CANDIDATE; } static inline bool will_be_unmounted(struct mount *m) @@ -464,7 +462,7 @@ static void umount_one(struct mount *m, struct list_head *to_umount) static void remove_from_candidate_list(struct mount *m) { - m->mnt.mnt_flags &= ~(MNT_MARKED | MNT_UMOUNT_CANDIDATE); + m->mnt_t_flags &= ~(T_MARKED | T_UMOUNT_CANDIDATE); list_del_init(&m->mnt_list); } @@ -476,7 +474,7 @@ static void gather_candidates(struct list_head *set, list_for_each_entry(m, set, mnt_list) { if (is_candidate(m)) continue; - m->mnt.mnt_flags |= MNT_UMOUNT_CANDIDATE; + m->mnt_t_flags |= T_UMOUNT_CANDIDATE; p = m->mnt_parent; q = propagation_next(p, p); while (q) { @@ -494,7 +492,7 @@ static void gather_candidates(struct list_head *set, q = skip_propagation_subtree(q, p); continue; } - child->mnt.mnt_flags |= MNT_UMOUNT_CANDIDATE; + child->mnt_t_flags |= T_UMOUNT_CANDIDATE; if (!will_be_unmounted(child)) list_add(&child->mnt_list, candidates); } @@ -502,7 +500,7 @@ static void gather_candidates(struct list_head *set, } } list_for_each_entry(m, set, mnt_list) - m->mnt.mnt_flags &= ~MNT_UMOUNT_CANDIDATE; + m->mnt_t_flags &= ~T_UMOUNT_CANDIDATE; } /* @@ -519,7 +517,7 @@ static void trim_ancestors(struct mount *m) return; SET_MNT_MARK(m); if (m != p->overmount) - p->mnt.mnt_flags &= ~MNT_UMOUNT_CANDIDATE; + p->mnt_t_flags &= ~T_UMOUNT_CANDIDATE; } } diff --git a/fs/pnode.h b/fs/pnode.h index 04f1ac53aa49..507e30e7a420 100644 --- a/fs/pnode.h +++ b/fs/pnode.h @@ -10,14 +10,14 @@ #include #include "mount.h" -#define IS_MNT_SHARED(m) ((m)->mnt.mnt_flags & MNT_SHARED) +#define IS_MNT_SHARED(m) ((m)->mnt_t_flags & T_SHARED) #define IS_MNT_SLAVE(m) ((m)->mnt_master) #define IS_MNT_NEW(m) (!(m)->mnt_ns) -#define CLEAR_MNT_SHARED(m) ((m)->mnt.mnt_flags &= ~MNT_SHARED) -#define IS_MNT_UNBINDABLE(m) ((m)->mnt.mnt_flags & MNT_UNBINDABLE) -#define IS_MNT_MARKED(m) ((m)->mnt.mnt_flags & MNT_MARKED) -#define SET_MNT_MARK(m) ((m)->mnt.mnt_flags |= MNT_MARKED) -#define CLEAR_MNT_MARK(m) ((m)->mnt.mnt_flags &= ~MNT_MARKED) +#define CLEAR_MNT_SHARED(m) ((m)->mnt_t_flags &= ~T_SHARED) +#define IS_MNT_UNBINDABLE(m) ((m)->mnt_t_flags & T_UNBINDABLE) +#define IS_MNT_MARKED(m) ((m)->mnt_t_flags & T_MARKED) +#define SET_MNT_MARK(m) ((m)->mnt_t_flags |= T_MARKED) +#define CLEAR_MNT_MARK(m) ((m)->mnt_t_flags &= ~T_MARKED) #define IS_MNT_LOCKED(m) ((m)->mnt.mnt_flags & MNT_LOCKED) #define CL_EXPIRE 0x01 @@ -28,10 +28,13 @@ #define CL_SHARED_TO_SLAVE 0x20 #define CL_COPY_MNT_NS_FILE 0x40 +/* + * EXCL[namespace_sem] + */ static inline void set_mnt_shared(struct mount *mnt) { - mnt->mnt.mnt_flags &= ~MNT_SHARED_MASK; - mnt->mnt.mnt_flags |= MNT_SHARED; + mnt->mnt_t_flags &= ~T_SHARED_MASK; + mnt->mnt_t_flags |= T_SHARED; } static inline bool peers(const struct mount *m1, const struct mount *m2) -- cgit v1.2.3 From 25776a09d802f4e4d8c0bd72042934223286c2e3 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 21 Jun 2025 17:41:40 -0400 Subject: propagate_one(): get rid of dest_master propagate_mnt() takes the subtree we are about to attach and creates its copies, setting the propagation between those. Each copy is cloned either from the original or from one of the already created copies. The tricky part is choosing the right copy to serve as a master when we are starting a new peer group. The algorithm for doing that selection puts temporary marks on the masters of mountpoints that already got a copy created for them; since the initial peer group might have no master at all, we need to special-case that when looking for the mark. Currently we do that by memorizing the master of original peer group. It works, but we get yet another piece of data to pass from propagate_mnt() to propagate_one(). Alternative is to mark the master of original peer group if not NULL, turning the check into "master is NULL or marked". Less data to pass around and memory safety is more obvious that way... Signed-off-by: Al Viro --- fs/pnode.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index b997663de6d0..870ebced10aa 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -215,7 +215,7 @@ static struct mount *next_group(struct mount *m, struct mount *origin) } /* all accesses are serialized by namespace_sem */ -static struct mount *last_dest, *first_source, *last_source, *dest_master; +static struct mount *last_dest, *first_source, *last_source; static struct hlist_head *list; static int propagate_one(struct mount *m, struct mountpoint *dest_mp) @@ -239,7 +239,7 @@ static int propagate_one(struct mount *m, struct mountpoint *dest_mp) bool done; for (n = m; ; n = p) { p = n->mnt_master; - if (p == dest_master || IS_MNT_MARKED(p)) + if (!p || IS_MNT_MARKED(p)) break; } do { @@ -264,7 +264,7 @@ static int propagate_one(struct mount *m, struct mountpoint *dest_mp) read_seqlock_excl(&mount_lock); mnt_set_mountpoint(m, dest_mp, child); read_sequnlock_excl(&mount_lock); - if (m->mnt_master != dest_master) + if (m->mnt_master) SET_MNT_MARK(m->mnt_master); last_dest = m; last_source = child; @@ -300,7 +300,8 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, first_source = source_mnt; last_source = source_mnt; list = tree_list; - dest_master = dest_mnt->mnt_master; + if (dest_mnt->mnt_master) + SET_MNT_MARK(dest_mnt->mnt_master); /* all peers of dest_mnt, except dest_mnt itself */ for (n = next_peer(dest_mnt); n != dest_mnt; n = next_peer(n)) { @@ -324,9 +325,11 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, out: hlist_for_each_entry(n, tree_list, mnt_hash) { m = n->mnt_parent; - if (m->mnt_master != dest_mnt->mnt_master) + if (m->mnt_master) CLEAR_MNT_MARK(m->mnt_master); } + if (dest_mnt->mnt_master) + CLEAR_MNT_MARK(dest_mnt->mnt_master); return ret; } -- cgit v1.2.3 From 2b2a34793dc239b0eaebe9559557d051524a7297 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 27 Jun 2025 22:56:43 -0400 Subject: propagate_mnt(): handle all peer groups in the same loop the only difference is that for the original group we want to skip the first element; not worth having the logics twice... Signed-off-by: Al Viro --- fs/pnode.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index 870ebced10aa..f55295e26217 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -289,7 +289,7 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, struct mount *source_mnt, struct hlist_head *tree_list) { struct mount *m, *n; - int ret = 0; + int err = 0; /* * we don't want to bother passing tons of arguments to @@ -303,26 +303,23 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, if (dest_mnt->mnt_master) SET_MNT_MARK(dest_mnt->mnt_master); - /* all peers of dest_mnt, except dest_mnt itself */ - for (n = next_peer(dest_mnt); n != dest_mnt; n = next_peer(n)) { - ret = propagate_one(n, dest_mp); - if (ret) - goto out; - } - - /* all slave groups */ - for (m = next_group(dest_mnt, dest_mnt); m; - m = next_group(m, dest_mnt)) { - /* everything in that slave group */ - n = m; + /* iterate over peer groups, depth first */ + for (m = dest_mnt; m && !err; m = next_group(m, dest_mnt)) { + if (m == dest_mnt) { // have one for dest_mnt itself + n = next_peer(m); + if (n == m) + continue; + } else { + n = m; + } do { - ret = propagate_one(n, dest_mp); - if (ret) - goto out; + err = propagate_one(n, dest_mp); + if (err) + break; n = next_peer(n); } while (n != m); } -out: + hlist_for_each_entry(n, tree_list, mnt_hash) { m = n->mnt_parent; if (m->mnt_master) @@ -330,7 +327,7 @@ out: } if (dest_mnt->mnt_master) CLEAR_MNT_MARK(dest_mnt->mnt_master); - return ret; + return err; } /* -- cgit v1.2.3 From 15e710b8bbb5c537c39ccaa23963d01c76946834 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 27 Jun 2025 23:04:23 -0400 Subject: propagate_one(): separate the "do we need secondary here?" logics take the checks into separate helper - need_secondary(mount, mountpoint). Signed-off-by: Al Viro --- fs/pnode.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index f55295e26217..7c832f98595c 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -218,19 +218,24 @@ static struct mount *next_group(struct mount *m, struct mount *origin) static struct mount *last_dest, *first_source, *last_source; static struct hlist_head *list; -static int propagate_one(struct mount *m, struct mountpoint *dest_mp) +static bool need_secondary(struct mount *m, struct mountpoint *dest_mp) { - struct mount *child; - int type; /* skip ones added by this propagate_mnt() */ if (IS_MNT_NEW(m)) - return 0; + return false; /* skip if mountpoint isn't visible in m */ if (!is_subdir(dest_mp->m_dentry, m->mnt.mnt_root)) - return 0; + return false; /* skip if m is in the anon_ns */ if (is_anon_ns(m->mnt_ns)) - return 0; + return false; + return true; +} + +static int propagate_one(struct mount *m, struct mountpoint *dest_mp) +{ + struct mount *child; + int type; if (peers(m, last_dest)) { type = CL_MAKE_SHARED; @@ -313,11 +318,12 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, n = m; } do { + if (!need_secondary(n, dest_mp)) + continue; err = propagate_one(n, dest_mp); if (err) break; - n = next_peer(n); - } while (n != m); + } while ((n = next_peer(n)) != m); } hlist_for_each_entry(n, tree_list, mnt_hash) { -- cgit v1.2.3 From e0f9396e244c0dcc29921ebc3254cb25d6eb31cf Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 27 Jun 2025 23:09:47 -0400 Subject: propagate_one(): separate the "what should be the master for this copy" part When we create the first copy for a peer group, it becomes a slave of one of the existing copies; take that logics into a separate helper - find_master(parent, last_copy, original). Signed-off-by: Al Viro --- fs/pnode.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index 7c832f98595c..94de8aad4da5 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -232,6 +232,31 @@ static bool need_secondary(struct mount *m, struct mountpoint *dest_mp) return true; } +static struct mount *find_master(struct mount *m, + struct mount *last_copy, + struct mount *original) +{ + struct mount *p; + + // ascend until there's a copy for something with the same master + for (;;) { + p = m->mnt_master; + if (!p || IS_MNT_MARKED(p)) + break; + m = p; + } + while (!peers(last_copy, original)) { + struct mount *parent = last_copy->mnt_parent; + if (parent->mnt_master == p) { + if (!peers(parent, m)) + last_copy = last_copy->mnt_master; + break; + } + last_copy = last_copy->mnt_master; + } + return last_copy; +} + static int propagate_one(struct mount *m, struct mountpoint *dest_mp) { struct mount *child; @@ -240,23 +265,7 @@ static int propagate_one(struct mount *m, struct mountpoint *dest_mp) if (peers(m, last_dest)) { type = CL_MAKE_SHARED; } else { - struct mount *n, *p; - bool done; - for (n = m; ; n = p) { - p = n->mnt_master; - if (!p || IS_MNT_MARKED(p)) - break; - } - do { - struct mount *parent = last_source->mnt_parent; - if (peers(last_source, first_source)) - break; - done = parent->mnt_master == p; - if (done && peers(n, parent)) - break; - last_source = last_source->mnt_master; - } while (!done); - + last_source = find_master(m, last_source, first_source); type = CL_SLAVE; /* beginning of peer group among the slaves? */ if (IS_MNT_SHARED(m)) -- cgit v1.2.3 From 6a2ce2a74bfeee4b66411177125f3b5749003e7f Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 27 Jun 2025 23:16:52 -0400 Subject: propagate_one(): fold into the sole caller mechanical expansion; will be cleaned up on the next step Signed-off-by: Al Viro --- fs/pnode.c | 57 +++++++++++++++++++++++++-------------------------------- 1 file changed, 25 insertions(+), 32 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index 94de8aad4da5..aeaec24f7456 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -257,35 +257,6 @@ static struct mount *find_master(struct mount *m, return last_copy; } -static int propagate_one(struct mount *m, struct mountpoint *dest_mp) -{ - struct mount *child; - int type; - - if (peers(m, last_dest)) { - type = CL_MAKE_SHARED; - } else { - last_source = find_master(m, last_source, first_source); - type = CL_SLAVE; - /* beginning of peer group among the slaves? */ - if (IS_MNT_SHARED(m)) - type |= CL_MAKE_SHARED; - } - - child = copy_tree(last_source, last_source->mnt.mnt_root, type); - if (IS_ERR(child)) - return PTR_ERR(child); - read_seqlock_excl(&mount_lock); - mnt_set_mountpoint(m, dest_mp, child); - read_sequnlock_excl(&mount_lock); - if (m->mnt_master) - SET_MNT_MARK(m->mnt_master); - last_dest = m; - last_source = child; - hlist_add_head(&child->mnt_hash, list); - return count_mounts(m->mnt_ns, child); -} - /* * mount 'source_mnt' under the destination 'dest_mnt' at * dentry 'dest_dentry'. And propagate that mount to @@ -302,8 +273,8 @@ static int propagate_one(struct mount *m, struct mountpoint *dest_mp) int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, struct mount *source_mnt, struct hlist_head *tree_list) { - struct mount *m, *n; - int err = 0; + struct mount *m, *n, *child; + int err = 0, type; /* * we don't want to bother passing tons of arguments to @@ -329,7 +300,29 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, do { if (!need_secondary(n, dest_mp)) continue; - err = propagate_one(n, dest_mp); + if (peers(n, last_dest)) { + type = CL_MAKE_SHARED; + } else { + last_source = find_master(n, last_source, first_source); + type = CL_SLAVE; + /* beginning of peer group among the slaves? */ + if (IS_MNT_SHARED(n)) + type |= CL_MAKE_SHARED; + } + child = copy_tree(last_source, last_source->mnt.mnt_root, type); + if (IS_ERR(child)) { + err = PTR_ERR(child); + break; + } + read_seqlock_excl(&mount_lock); + mnt_set_mountpoint(n, dest_mp, child); + read_sequnlock_excl(&mount_lock); + if (n->mnt_master) + SET_MNT_MARK(n->mnt_master); + last_dest = n; + last_source = child; + hlist_add_head(&child->mnt_hash, list); + err = count_mounts(n->mnt_ns, child); if (err) break; } while ((n = next_peer(n)) != m); -- cgit v1.2.3 From bc88530a20b1d5c78288ef5383d10b66d3242c48 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 27 Jun 2025 23:21:57 -0400 Subject: fs/pnode.c: get rid of globals this stuff can be local in propagate_mnt() now (and in some cases duplicates the existing variables there) Signed-off-by: Al Viro --- fs/pnode.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index aeaec24f7456..e01f43820a93 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -214,10 +214,6 @@ static struct mount *next_group(struct mount *m, struct mount *origin) } } -/* all accesses are serialized by namespace_sem */ -static struct mount *last_dest, *first_source, *last_source; -static struct hlist_head *list; - static bool need_secondary(struct mount *m, struct mountpoint *dest_mp) { /* skip ones added by this propagate_mnt() */ @@ -273,18 +269,11 @@ static struct mount *find_master(struct mount *m, int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, struct mount *source_mnt, struct hlist_head *tree_list) { - struct mount *m, *n, *child; + struct mount *m, *n, *copy, *this, *last_dest; int err = 0, type; - /* - * we don't want to bother passing tons of arguments to - * propagate_one(); everything is serialized by namespace_sem, - * so globals will do just fine. - */ last_dest = dest_mnt; - first_source = source_mnt; - last_source = source_mnt; - list = tree_list; + copy = source_mnt; if (dest_mnt->mnt_master) SET_MNT_MARK(dest_mnt->mnt_master); @@ -303,26 +292,26 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, if (peers(n, last_dest)) { type = CL_MAKE_SHARED; } else { - last_source = find_master(n, last_source, first_source); + copy = find_master(n, copy, source_mnt); type = CL_SLAVE; /* beginning of peer group among the slaves? */ if (IS_MNT_SHARED(n)) type |= CL_MAKE_SHARED; } - child = copy_tree(last_source, last_source->mnt.mnt_root, type); - if (IS_ERR(child)) { - err = PTR_ERR(child); + this = copy_tree(copy, copy->mnt.mnt_root, type); + if (IS_ERR(this)) { + err = PTR_ERR(this); break; } read_seqlock_excl(&mount_lock); - mnt_set_mountpoint(n, dest_mp, child); + mnt_set_mountpoint(n, dest_mp, this); read_sequnlock_excl(&mount_lock); if (n->mnt_master) SET_MNT_MARK(n->mnt_master); last_dest = n; - last_source = child; - hlist_add_head(&child->mnt_hash, list); - err = count_mounts(n->mnt_ns, child); + copy = this; + hlist_add_head(&this->mnt_hash, tree_list); + err = count_mounts(n->mnt_ns, this); if (err) break; } while ((n = next_peer(n)) != m); -- cgit v1.2.3 From 0a10217e5cf82835b63875752b57f01bba0bf5b6 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 27 Jun 2025 23:27:48 -0400 Subject: propagate_mnt(): get rid of last_dest Its only use is choosing the type of copy - CL_MAKE_SHARED if there already is a copy in that peer group, CL_SLAVE or CL_SLAVE | CL_MAKE_SHARED otherwise. But that's easy to keep track of - just set type in the beginning of group and reset to CL_MAKE_SHARED after the first created secondary in it... Signed-off-by: Al Viro --- fs/pnode.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index e01f43820a93..b3af55123a82 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -269,35 +269,32 @@ static struct mount *find_master(struct mount *m, int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, struct mount *source_mnt, struct hlist_head *tree_list) { - struct mount *m, *n, *copy, *this, *last_dest; + struct mount *m, *n, *copy, *this; int err = 0, type; - last_dest = dest_mnt; - copy = source_mnt; if (dest_mnt->mnt_master) SET_MNT_MARK(dest_mnt->mnt_master); /* iterate over peer groups, depth first */ for (m = dest_mnt; m && !err; m = next_group(m, dest_mnt)) { if (m == dest_mnt) { // have one for dest_mnt itself + copy = source_mnt; + type = CL_MAKE_SHARED; n = next_peer(m); if (n == m) continue; } else { + type = CL_SLAVE; + /* beginning of peer group among the slaves? */ + if (IS_MNT_SHARED(m)) + type |= CL_MAKE_SHARED; n = m; } do { if (!need_secondary(n, dest_mp)) continue; - if (peers(n, last_dest)) { - type = CL_MAKE_SHARED; - } else { + if (type & CL_SLAVE) // first in this peer group copy = find_master(n, copy, source_mnt); - type = CL_SLAVE; - /* beginning of peer group among the slaves? */ - if (IS_MNT_SHARED(n)) - type |= CL_MAKE_SHARED; - } this = copy_tree(copy, copy->mnt.mnt_root, type); if (IS_ERR(this)) { err = PTR_ERR(this); @@ -308,12 +305,12 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, read_sequnlock_excl(&mount_lock); if (n->mnt_master) SET_MNT_MARK(n->mnt_master); - last_dest = n; copy = this; hlist_add_head(&this->mnt_hash, tree_list); err = count_mounts(n->mnt_ns, this); if (err) break; + type = CL_MAKE_SHARED; } while ((n = next_peer(n)) != m); } -- cgit v1.2.3 From 0313356520b15deab893cd62da3e2ba9a6e61a1f Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 27 Jun 2025 23:39:23 -0400 Subject: propagate_mnt(): fix comment and convert to kernel-doc, while we are at it Mountpoint is passed as struct mountpoint *, not struct dentry * (and called dest_mp, not dest_dentry) since 2013. Roots of created copies are linked via mnt_hash, not mnt_list since a bit before the merge into mainline back in 2005. Signed-off-by: Al Viro --- fs/pnode.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index b3af55123a82..b887116f0041 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -253,21 +253,20 @@ static struct mount *find_master(struct mount *m, return last_copy; } -/* - * mount 'source_mnt' under the destination 'dest_mnt' at - * dentry 'dest_dentry'. And propagate that mount to - * all the peer and slave mounts of 'dest_mnt'. - * Link all the new mounts into a propagation tree headed at - * source_mnt. Also link all the new mounts using ->mnt_list - * headed at source_mnt's ->mnt_list +/** + * propagate_mnt() - create secondary copies for tree attachment + * @dest_mnt: destination mount. + * @dest_mp: destination mountpoint. + * @source_mnt: source mount. + * @tree_list: list of secondaries to be attached. * - * @dest_mnt: destination mount. - * @dest_dentry: destination dentry. - * @source_mnt: source mount. - * @tree_list : list of heads of trees to be attached. + * Create secondary copies for attaching a tree with root @source_mnt + * at mount @dest_mnt with mountpoint @dest_mp. Link all new mounts + * into a propagation graph. Set mountpoints for all secondaries, + * link their roots into @tree_list via ->mnt_hash. */ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, - struct mount *source_mnt, struct hlist_head *tree_list) + struct mount *source_mnt, struct hlist_head *tree_list) { struct mount *m, *n, *copy, *this; int err = 0, type; -- cgit v1.2.3 From d5f15047f13b86b74d1ac4f39036ccae2078c492 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 24 Jun 2025 23:20:47 -0400 Subject: change_mnt_propagation() cleanups, step 1 Lift changing ->mnt_slave from do_make_slave() into the caller. Simplifies the next steps... Signed-off-by: Al Viro --- fs/pnode.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index b887116f0041..14618eac2025 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -104,7 +104,6 @@ static int do_make_slave(struct mount *mnt) } list_for_each_entry(slave_mnt, &mnt->mnt_slave_list, mnt_slave) slave_mnt->mnt_master = master; - list_move(&mnt->mnt_slave, &master->mnt_slave_list); list_splice(&mnt->mnt_slave_list, master->mnt_slave_list.prev); INIT_LIST_HEAD(&mnt->mnt_slave_list); mnt->mnt_master = master; @@ -121,8 +120,12 @@ void change_mnt_propagation(struct mount *mnt, int type) return; } do_make_slave(mnt); - if (type != MS_SLAVE) { - list_del_init(&mnt->mnt_slave); + list_del_init(&mnt->mnt_slave); + if (type == MS_SLAVE) { + if (mnt->mnt_master) + list_add(&mnt->mnt_slave, + &mnt->mnt_master->mnt_slave_list); + } else { mnt->mnt_master = NULL; if (type == MS_UNBINDABLE) mnt->mnt_t_flags |= T_UNBINDABLE; -- cgit v1.2.3 From ef86251194de9b31f7efcf70ea480ebe736e9e60 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 24 Jun 2025 23:25:00 -0400 Subject: change_mnt_propagation(): do_make_slave() is a no-op unless IS_MNT_SHARED() ... since mnt->mnt_share and mnt->mnt_slave_list are guaranteed to be empty unless IS_MNT_SHARED(mnt). Signed-off-by: Al Viro --- fs/pnode.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index 14618eac2025..9723f05cda5f 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -70,10 +70,8 @@ static int do_make_slave(struct mount *mnt) struct mount *master, *slave_mnt; if (list_empty(&mnt->mnt_share)) { - if (IS_MNT_SHARED(mnt)) { - mnt_release_group_id(mnt); - CLEAR_MNT_SHARED(mnt); - } + mnt_release_group_id(mnt); + CLEAR_MNT_SHARED(mnt); master = mnt->mnt_master; if (!master) { struct list_head *p = &mnt->mnt_slave_list; @@ -119,7 +117,8 @@ void change_mnt_propagation(struct mount *mnt, int type) set_mnt_shared(mnt); return; } - do_make_slave(mnt); + if (IS_MNT_SHARED(mnt)) + do_make_slave(mnt); list_del_init(&mnt->mnt_slave); if (type == MS_SLAVE) { if (mnt->mnt_master) -- cgit v1.2.3 From 955336e204ab59301ff8b1f75a98a226f5a98782 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 24 Jun 2025 18:12:56 -0400 Subject: do_make_slave(): choose new master sanely When mount changes propagation type so that it doesn't propagate events any more (MS_PRIVATE, MS_SLAVE, MS_UNBINDABLE), we need to make sure that event propagation between other mounts is unaffected. We need to make sure that events from peers and master of that mount (if any) still reach everything that used to be on its ->mnt_slave_list. If mount has neither peers nor master, we simply need to dissolve its ->mnt_slave_list and clear ->mnt_master of everything in there. If mount has peers, we transfer everything in ->mnt_slave_list of this mount into that of some of those peers (and adjust ->mnt_master accordingly). If mount has a master but no peers, we transfer everything in ->mnt_slave_list of this mount into that of its master (adjusting ->mnt_master, etc.). There are two problems with the current implementation: * there's a long-obsolete logics in choosing the peer - once upon a time it made sense to prefer the peer that had the same ->mnt_root as our mount, but that had been pointless since 2014 ("smarter propagate_mnt()") * the most common caller of that thing is umount_tree() taking the mounts out of propagation graph. In that case it's possible to have ->mnt_slave_list contents moved many times, since the replacement master is likely to be taken out by the same umount_tree(), etc. Take the choice of replacement master into a separate function (propagation_source()) and teach it to skip the candidates that are going to be taken out. Signed-off-by: Al Viro --- fs/pnode.c | 62 +++++++++++++++++++++++++++++++------------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index 9723f05cda5f..91d10af867bd 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -65,40 +65,45 @@ int get_dominating_id(struct mount *mnt, const struct path *root) return 0; } +static inline bool will_be_unmounted(struct mount *m) +{ + return m->mnt.mnt_flags & MNT_UMOUNT; +} + +static struct mount *propagation_source(struct mount *mnt) +{ + do { + struct mount *m; + for (m = next_peer(mnt); m != mnt; m = next_peer(m)) { + if (!will_be_unmounted(m)) + return m; + } + mnt = mnt->mnt_master; + } while (mnt && will_be_unmounted(mnt)); + return mnt; +} + static int do_make_slave(struct mount *mnt) { - struct mount *master, *slave_mnt; + struct mount *master = propagation_source(mnt); + struct mount *slave_mnt; if (list_empty(&mnt->mnt_share)) { mnt_release_group_id(mnt); - CLEAR_MNT_SHARED(mnt); - master = mnt->mnt_master; - if (!master) { - struct list_head *p = &mnt->mnt_slave_list; - while (!list_empty(p)) { - slave_mnt = list_first_entry(p, - struct mount, mnt_slave); - list_del_init(&slave_mnt->mnt_slave); - slave_mnt->mnt_master = NULL; - } - return 0; - } } else { - struct mount *m; - /* - * slave 'mnt' to a peer mount that has the - * same root dentry. If none is available then - * slave it to anything that is available. - */ - for (m = master = next_peer(mnt); m != mnt; m = next_peer(m)) { - if (m->mnt.mnt_root == mnt->mnt.mnt_root) { - master = m; - break; - } - } list_del_init(&mnt->mnt_share); mnt->mnt_group_id = 0; - CLEAR_MNT_SHARED(mnt); + } + CLEAR_MNT_SHARED(mnt); + if (!master) { + struct list_head *p = &mnt->mnt_slave_list; + while (!list_empty(p)) { + slave_mnt = list_first_entry(p, + struct mount, mnt_slave); + list_del_init(&slave_mnt->mnt_slave); + slave_mnt->mnt_master = NULL; + } + return 0; } list_for_each_entry(slave_mnt, &mnt->mnt_slave_list, mnt_slave) slave_mnt->mnt_master = master; @@ -443,11 +448,6 @@ static inline bool is_candidate(struct mount *m) return m->mnt_t_flags & T_UMOUNT_CANDIDATE; } -static inline bool will_be_unmounted(struct mount *m) -{ - return m->mnt.mnt_flags & MNT_UMOUNT; -} - static void umount_one(struct mount *m, struct list_head *to_umount) { m->mnt.mnt_flags |= MNT_UMOUNT; -- cgit v1.2.3 From 94a8d0027606397ce58b00077bf6146f25923965 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 24 Jun 2025 23:36:43 -0400 Subject: turn do_make_slave() into transfer_propagation() Lift calculation of replacement propagation source, removal from peer group and assignment of ->mnt_master from do_make_slave() into change_mnt_propagation() itself. What remains is switching of what used to get propagation *through* mnt to alternative source. Rename to transfer_propagation(), passing it the replacement source as the second argument. Have it return void, while we are at it. Signed-off-by: Al Viro --- fs/pnode.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index 91d10af867bd..0a54848cbbd1 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -83,19 +83,10 @@ static struct mount *propagation_source(struct mount *mnt) return mnt; } -static int do_make_slave(struct mount *mnt) +static void transfer_propagation(struct mount *mnt, struct mount *to) { - struct mount *master = propagation_source(mnt); struct mount *slave_mnt; - - if (list_empty(&mnt->mnt_share)) { - mnt_release_group_id(mnt); - } else { - list_del_init(&mnt->mnt_share); - mnt->mnt_group_id = 0; - } - CLEAR_MNT_SHARED(mnt); - if (!master) { + if (!to) { struct list_head *p = &mnt->mnt_slave_list; while (!list_empty(p)) { slave_mnt = list_first_entry(p, @@ -103,14 +94,12 @@ static int do_make_slave(struct mount *mnt) list_del_init(&slave_mnt->mnt_slave); slave_mnt->mnt_master = NULL; } - return 0; + return; } list_for_each_entry(slave_mnt, &mnt->mnt_slave_list, mnt_slave) - slave_mnt->mnt_master = master; - list_splice(&mnt->mnt_slave_list, master->mnt_slave_list.prev); + slave_mnt->mnt_master = to; + list_splice(&mnt->mnt_slave_list, to->mnt_slave_list.prev); INIT_LIST_HEAD(&mnt->mnt_slave_list); - mnt->mnt_master = master; - return 0; } /* @@ -122,8 +111,19 @@ void change_mnt_propagation(struct mount *mnt, int type) set_mnt_shared(mnt); return; } - if (IS_MNT_SHARED(mnt)) - do_make_slave(mnt); + if (IS_MNT_SHARED(mnt)) { + struct mount *m = propagation_source(mnt); + + if (list_empty(&mnt->mnt_share)) { + mnt_release_group_id(mnt); + } else { + list_del_init(&mnt->mnt_share); + mnt->mnt_group_id = 0; + } + CLEAR_MNT_SHARED(mnt); + transfer_propagation(mnt, m); + mnt->mnt_master = m; + } list_del_init(&mnt->mnt_slave); if (type == MS_SLAVE) { if (mnt->mnt_master) -- cgit v1.2.3 From 8c5a853f58c5b86b033842b78a0ad3d1208672fa Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 24 Jun 2025 23:51:31 -0400 Subject: mnt_slave_list/mnt_slave: turn into hlist_head/hlist_node Signed-off-by: Al Viro --- fs/mount.h | 4 ++-- fs/namespace.c | 14 ++++++-------- fs/pnode.c | 41 +++++++++++++++++++---------------------- 3 files changed, 27 insertions(+), 32 deletions(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index f299dc85446d..08583428b10b 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -69,8 +69,8 @@ struct mount { struct list_head mnt_list; struct list_head mnt_expire; /* link in fs-specific expiry list */ struct list_head mnt_share; /* circular list of shared mounts */ - struct list_head mnt_slave_list;/* list of slave mounts */ - struct list_head mnt_slave; /* slave list entry */ + struct hlist_head mnt_slave_list;/* list of slave mounts */ + struct hlist_node mnt_slave; /* slave list entry */ struct mount *mnt_master; /* slave is on master->mnt_slave_list */ struct mnt_namespace *mnt_ns; /* containing namespace */ struct mountpoint *mnt_mp; /* where is it mounted */ diff --git a/fs/namespace.c b/fs/namespace.c index da27365418a5..38a46b32413d 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -380,8 +380,8 @@ static struct mount *alloc_vfsmnt(const char *name) INIT_LIST_HEAD(&mnt->mnt_list); INIT_LIST_HEAD(&mnt->mnt_expire); INIT_LIST_HEAD(&mnt->mnt_share); - INIT_LIST_HEAD(&mnt->mnt_slave_list); - INIT_LIST_HEAD(&mnt->mnt_slave); + INIT_HLIST_HEAD(&mnt->mnt_slave_list); + INIT_HLIST_NODE(&mnt->mnt_slave); INIT_HLIST_NODE(&mnt->mnt_mp_list); INIT_HLIST_HEAD(&mnt->mnt_stuck_children); RB_CLEAR_NODE(&mnt->mnt_node); @@ -1348,10 +1348,10 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, if ((flag & CL_SLAVE) || ((flag & CL_SHARED_TO_SLAVE) && IS_MNT_SHARED(old))) { - list_add(&mnt->mnt_slave, &old->mnt_slave_list); + hlist_add_head(&mnt->mnt_slave, &old->mnt_slave_list); mnt->mnt_master = old; } else if (IS_MNT_SLAVE(old)) { - list_add(&mnt->mnt_slave, &old->mnt_slave); + hlist_add_behind(&mnt->mnt_slave, &old->mnt_slave); mnt->mnt_master = old->mnt_master; } return mnt; @@ -3398,10 +3398,8 @@ static int do_set_group(struct path *from_path, struct path *to_path) goto out; if (IS_MNT_SLAVE(from)) { - struct mount *m = from->mnt_master; - - list_add(&to->mnt_slave, &from->mnt_slave); - to->mnt_master = m; + hlist_add_behind(&to->mnt_slave, &from->mnt_slave); + to->mnt_master = from->mnt_master; } if (IS_MNT_SHARED(from)) { diff --git a/fs/pnode.c b/fs/pnode.c index 0a54848cbbd1..69278079faeb 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -21,12 +21,12 @@ static inline struct mount *next_peer(struct mount *p) static inline struct mount *first_slave(struct mount *p) { - return list_entry(p->mnt_slave_list.next, struct mount, mnt_slave); + return hlist_entry(p->mnt_slave_list.first, struct mount, mnt_slave); } static inline struct mount *next_slave(struct mount *p) { - return list_entry(p->mnt_slave.next, struct mount, mnt_slave); + return hlist_entry(p->mnt_slave.next, struct mount, mnt_slave); } static struct mount *get_peer_under_root(struct mount *mnt, @@ -85,21 +85,18 @@ static struct mount *propagation_source(struct mount *mnt) static void transfer_propagation(struct mount *mnt, struct mount *to) { - struct mount *slave_mnt; - if (!to) { - struct list_head *p = &mnt->mnt_slave_list; - while (!list_empty(p)) { - slave_mnt = list_first_entry(p, - struct mount, mnt_slave); - list_del_init(&slave_mnt->mnt_slave); - slave_mnt->mnt_master = NULL; - } - return; + struct hlist_node *p = NULL, *n; + struct mount *m; + + hlist_for_each_entry_safe(m, n, &mnt->mnt_slave_list, mnt_slave) { + m->mnt_master = to; + if (!to) + hlist_del_init(&m->mnt_slave); + else + p = &m->mnt_slave; } - list_for_each_entry(slave_mnt, &mnt->mnt_slave_list, mnt_slave) - slave_mnt->mnt_master = to; - list_splice(&mnt->mnt_slave_list, to->mnt_slave_list.prev); - INIT_LIST_HEAD(&mnt->mnt_slave_list); + if (p) + hlist_splice_init(&mnt->mnt_slave_list, p, &to->mnt_slave_list); } /* @@ -124,10 +121,10 @@ void change_mnt_propagation(struct mount *mnt, int type) transfer_propagation(mnt, m); mnt->mnt_master = m; } - list_del_init(&mnt->mnt_slave); + hlist_del_init(&mnt->mnt_slave); if (type == MS_SLAVE) { if (mnt->mnt_master) - list_add(&mnt->mnt_slave, + hlist_add_head(&mnt->mnt_slave, &mnt->mnt_master->mnt_slave_list); } else { mnt->mnt_master = NULL; @@ -147,7 +144,7 @@ static struct mount *__propagation_next(struct mount *m, if (master == origin->mnt_master) { struct mount *next = next_peer(m); return (next == origin) ? NULL : next; - } else if (m->mnt_slave.next != &master->mnt_slave_list) + } else if (m->mnt_slave.next) return next_slave(m); /* back at master */ @@ -169,7 +166,7 @@ static struct mount *propagation_next(struct mount *m, struct mount *origin) { /* are there any slaves of this mount? */ - if (!IS_MNT_NEW(m) && !list_empty(&m->mnt_slave_list)) + if (!IS_MNT_NEW(m) && !hlist_empty(&m->mnt_slave_list)) return first_slave(m); return __propagation_next(m, origin); @@ -194,7 +191,7 @@ static struct mount *next_group(struct mount *m, struct mount *origin) while (1) { while (1) { struct mount *next; - if (!IS_MNT_NEW(m) && !list_empty(&m->mnt_slave_list)) + if (!IS_MNT_NEW(m) && !hlist_empty(&m->mnt_slave_list)) return first_slave(m); next = next_peer(m); if (m->mnt_group_id == origin->mnt_group_id) { @@ -207,7 +204,7 @@ static struct mount *next_group(struct mount *m, struct mount *origin) /* m is the last peer */ while (1) { struct mount *master = m->mnt_master; - if (m->mnt_slave.next != &master->mnt_slave_list) + if (m->mnt_slave.next) return next_slave(m); m = next_peer(master); if (master->mnt_group_id == origin->mnt_group_id) -- cgit v1.2.3 From dd5a4e1d640bf3542c4583491e6b91d25de3b760 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 24 Jun 2025 23:54:41 -0400 Subject: change_mnt_propagation(): move ->mnt_master assignment into MS_SLAVE case Signed-off-by: Al Viro --- fs/pnode.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/pnode.c b/fs/pnode.c index 69278079faeb..cbf5f5746252 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -104,13 +104,14 @@ static void transfer_propagation(struct mount *mnt, struct mount *to) */ void change_mnt_propagation(struct mount *mnt, int type) { + struct mount *m = mnt->mnt_master; + if (type == MS_SHARED) { set_mnt_shared(mnt); return; } if (IS_MNT_SHARED(mnt)) { - struct mount *m = propagation_source(mnt); - + m = propagation_source(mnt); if (list_empty(&mnt->mnt_share)) { mnt_release_group_id(mnt); } else { @@ -119,13 +120,12 @@ void change_mnt_propagation(struct mount *mnt, int type) } CLEAR_MNT_SHARED(mnt); transfer_propagation(mnt, m); - mnt->mnt_master = m; } hlist_del_init(&mnt->mnt_slave); if (type == MS_SLAVE) { - if (mnt->mnt_master) - hlist_add_head(&mnt->mnt_slave, - &mnt->mnt_master->mnt_slave_list); + mnt->mnt_master = m; + if (m) + hlist_add_head(&mnt->mnt_slave, &m->mnt_slave_list); } else { mnt->mnt_master = NULL; if (type == MS_UNBINDABLE) -- cgit v1.2.3 From 663206854f020ec6fc6bfd3d52f501a28ede1403 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 17 Jun 2025 21:35:22 -0400 Subject: copy_tree(): don't link the mounts via mnt_list The only place that really needs to be adjusted is commit_tree() - there we need to iterate through the copy and we might as well use next_mnt() for that. However, in case when our tree has been slid under something already mounted (propagation to a mountpoint that already has something mounted on it or a 'beneath' move_mount) we need to take care not to walk into the overmounting tree. Signed-off-by: Al Viro --- fs/mount.h | 3 +-- fs/namespace.c | 60 +++++++++++++++++++++++----------------------------------- fs/pnode.c | 3 ++- 3 files changed, 27 insertions(+), 39 deletions(-) (limited to 'fs') diff --git a/fs/mount.h b/fs/mount.h index 08583428b10b..97737051a8b9 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -193,7 +193,7 @@ static inline bool mnt_ns_empty(const struct mnt_namespace *ns) return RB_EMPTY_ROOT(&ns->mounts); } -static inline void move_from_ns(struct mount *mnt, struct list_head *dt_list) +static inline void move_from_ns(struct mount *mnt) { struct mnt_namespace *ns = mnt->mnt_ns; WARN_ON(!mnt_ns_attached(mnt)); @@ -203,7 +203,6 @@ static inline void move_from_ns(struct mount *mnt, struct list_head *dt_list) ns->mnt_first_node = rb_next(&mnt->mnt_node); rb_erase(&mnt->mnt_node, &ns->mounts); RB_CLEAR_NODE(&mnt->mnt_node); - list_add_tail(&mnt->mnt_list, dt_list); } bool has_locked_children(struct mount *mnt, struct dentry *dentry); diff --git a/fs/namespace.c b/fs/namespace.c index 38a46b32413d..bd6c7da901fc 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1161,34 +1161,6 @@ static void mnt_add_to_ns(struct mnt_namespace *ns, struct mount *mnt) mnt_notify_add(mnt); } -/* - * vfsmount lock must be held for write - */ -static void commit_tree(struct mount *mnt) -{ - struct mount *parent = mnt->mnt_parent; - struct mount *m; - LIST_HEAD(head); - struct mnt_namespace *n = parent->mnt_ns; - - BUG_ON(parent == mnt); - - if (!mnt_ns_attached(mnt)) { - list_add_tail(&head, &mnt->mnt_list); - while (!list_empty(&head)) { - m = list_first_entry(&head, typeof(*m), mnt_list); - list_del(&m->mnt_list); - - mnt_add_to_ns(n, m); - } - n->nr_mounts += n->pending_mounts; - n->pending_mounts = 0; - } - - make_visible(mnt); - touch_mnt_namespace(n); -} - static struct mount *next_mnt(struct mount *p, struct mount *root) { struct list_head *next = p->mnt_mounts.next; @@ -1215,6 +1187,27 @@ static struct mount *skip_mnt_tree(struct mount *p) return p; } +/* + * vfsmount lock must be held for write + */ +static void commit_tree(struct mount *mnt) +{ + struct mnt_namespace *n = mnt->mnt_parent->mnt_ns; + + if (!mnt_ns_attached(mnt)) { + for (struct mount *m = mnt; m; m = next_mnt(m, mnt)) + if (unlikely(mnt_ns_attached(m))) + m = skip_mnt_tree(m); + else + mnt_add_to_ns(n, m); + n->nr_mounts += n->pending_mounts; + n->pending_mounts = 0; + } + + make_visible(mnt); + touch_mnt_namespace(n); +} + /** * vfs_create_mount - Create a mount for a configured superblock * @fc: The configuration context with the superblock attached @@ -1831,9 +1824,8 @@ static void umount_tree(struct mount *mnt, enum umount_tree_flags how) for (p = mnt; p; p = next_mnt(p, mnt)) { p->mnt.mnt_flags |= MNT_UMOUNT; if (mnt_ns_attached(p)) - move_from_ns(p, &tmp_list); - else - list_move(&p->mnt_list, &tmp_list); + move_from_ns(p); + list_add_tail(&p->mnt_list, &tmp_list); } /* Hide the mounts from mnt_mounts */ @@ -2270,7 +2262,6 @@ struct mount *copy_tree(struct mount *src_root, struct dentry *dentry, list_add(&dst_mnt->mnt_expire, &src_mnt->mnt_expire); } - list_add_tail(&dst_mnt->mnt_list, &res->mnt_list); attach_mnt(dst_mnt, dst_parent, src_parent->mnt_mp); unlock_mount_hash(); } @@ -2686,12 +2677,9 @@ static int attach_recursive_mnt(struct mount *source_mnt, list_del_init(&source_mnt->mnt_expire); } else { if (source_mnt->mnt_ns) { - LIST_HEAD(head); - /* move from anon - the caller will destroy */ for (p = source_mnt; p; p = next_mnt(p, source_mnt)) - move_from_ns(p, &head); - list_del_init(&head); + move_from_ns(p); } } diff --git a/fs/pnode.c b/fs/pnode.c index cbf5f5746252..81f7599bdac4 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -449,7 +449,8 @@ static void umount_one(struct mount *m, struct list_head *to_umount) { m->mnt.mnt_flags |= MNT_UMOUNT; list_del_init(&m->mnt_child); - move_from_ns(m, to_umount); + move_from_ns(m); + list_add_tail(&m->mnt_list, to_umount); } static void remove_from_candidate_list(struct mount *m) -- cgit v1.2.3 From aab771f34e63ef89e195b63d121abcb55eebfde6 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 18 Jun 2025 18:23:41 -0400 Subject: take freeing of emptied mnt_namespace to namespace_unlock() Freeing of a namespace must be delayed until after we'd dealt with mount notifications (in namespace_unlock()). The reasons are not immediately obvious (they are buried in ->prev_ns handling in mnt_notify()), and having that free_mnt_ns() explicitly called after namespace_unlock() is asking for trouble - it does feel like they should be OK to free as soon as they've been emptied. Make the things more explicit by setting 'emptied_ns' under namespace_sem and having namespace_unlock() free the sucker as soon as it's safe to free. Signed-off-by: Al Viro --- fs/namespace.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index bd6c7da901fc..85db0de5fb53 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -79,6 +79,7 @@ static struct kmem_cache *mnt_cache __ro_after_init; static DECLARE_RWSEM(namespace_sem); static HLIST_HEAD(unmounted); /* protected by namespace_sem */ static LIST_HEAD(ex_mountpoints); /* protected by namespace_sem */ +static struct mnt_namespace *emptied_ns; /* protected by namespace_sem */ static DEFINE_SEQLOCK(mnt_ns_tree_lock); #ifdef CONFIG_FSNOTIFY @@ -1730,15 +1731,18 @@ static bool need_notify_mnt_list(void) } #endif +static void free_mnt_ns(struct mnt_namespace *); static void namespace_unlock(void) { struct hlist_head head; struct hlist_node *p; struct mount *m; + struct mnt_namespace *ns = emptied_ns; LIST_HEAD(list); hlist_move_list(&unmounted, &head); list_splice_init(&ex_mountpoints, &list); + emptied_ns = NULL; if (need_notify_mnt_list()) { /* @@ -1752,6 +1756,11 @@ static void namespace_unlock(void) } else { up_write(&namespace_sem); } + if (unlikely(ns)) { + /* Make sure we notice when we leak mounts. */ + VFS_WARN_ON_ONCE(!mnt_ns_empty(ns)); + free_mnt_ns(ns); + } shrink_dentry_list(&list); @@ -2335,12 +2344,10 @@ void drop_collected_paths(struct path *paths, struct path *prealloc) kfree(paths); } -static void free_mnt_ns(struct mnt_namespace *); static struct mnt_namespace *alloc_mnt_ns(struct user_namespace *, bool); void dissolve_on_fput(struct vfsmount *mnt) { - struct mnt_namespace *ns; struct mount *m = real_mount(mnt); /* @@ -2362,15 +2369,11 @@ void dissolve_on_fput(struct vfsmount *mnt) if (!anon_ns_root(m)) return; - ns = m->mnt_ns; + emptied_ns = m->mnt_ns; lock_mount_hash(); umount_tree(m, UMOUNT_CONNECTED); unlock_mount_hash(); } - - /* Make sure we notice when we leak mounts. */ - VFS_WARN_ON_ONCE(!mnt_ns_empty(ns)); - free_mnt_ns(ns); } static bool __has_locked_children(struct mount *mnt, struct dentry *dentry) @@ -2678,6 +2681,7 @@ static int attach_recursive_mnt(struct mount *source_mnt, } else { if (source_mnt->mnt_ns) { /* move from anon - the caller will destroy */ + emptied_ns = source_mnt->mnt_ns; for (p = source_mnt; p; p = next_mnt(p, source_mnt)) move_from_ns(p); } @@ -3656,13 +3660,6 @@ static int do_move_mount(struct path *old_path, err = attach_recursive_mnt(old, p, mp.mp); out: unlock_mount(&mp); - if (!err) { - if (is_anon_ns(ns)) { - /* Make sure we notice when we leak mounts. */ - VFS_WARN_ON_ONCE(!mnt_ns_empty(ns)); - free_mnt_ns(ns); - } - } return err; } @@ -6153,11 +6150,11 @@ void put_mnt_ns(struct mnt_namespace *ns) if (!refcount_dec_and_test(&ns->ns.count)) return; namespace_lock(); + emptied_ns = ns; lock_mount_hash(); umount_tree(ns->root, 0); unlock_mount_hash(); namespace_unlock(); - free_mnt_ns(ns); } struct vfsmount *kern_mount(struct file_system_type *type) -- cgit v1.2.3 From 725ab435ff6e31faca26b8234f9f04c19f772b18 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 25 Jun 2025 14:48:50 -0400 Subject: get rid of CL_SHARE_TO_SLAVE the only difference between it and CL_SLAVE is in this predicate in clone_mnt(): if ((flag & CL_SLAVE) || ((flag & CL_SHARED_TO_SLAVE) && IS_MNT_SHARED(old))) { However, in case of CL_SHARED_TO_SLAVE we have not allocated any mount group ids since the time we'd grabbed namespace_sem, so IS_MNT_SHARED() is equivalent to non-zero ->mnt_group_id. And in case of CL_SLAVE old has come either from the original tree, which had ->mnt_group_id allocated for all nodes or from result of sequence of CL_MAKE_SHARED or CL_MAKE_SHARED|CL_SLAVE copies, ultimately going back to the original tree. In both cases we are guaranteed that old->mnt_group_id will be non-zero. In other words, the predicate is always equal to (flags & (CL_SLAVE | CL_SHARED_TO_SLAVE)) && old->mnt_group_id and with that replacement CL_SLAVE and CL_SHARED_TO_SLAVE have exact same behaviour. Signed-off-by: Al Viro --- fs/namespace.c | 7 +++---- fs/pnode.h | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index 85db0de5fb53..ca36c4a6a143 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1309,7 +1309,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; - if (flag & (CL_SLAVE | CL_PRIVATE | CL_SHARED_TO_SLAVE)) + if (flag & (CL_SLAVE | CL_PRIVATE)) mnt->mnt_group_id = 0; /* not a peer of original */ else mnt->mnt_group_id = old->mnt_group_id; @@ -1340,8 +1340,7 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, if (peers(mnt, old)) list_add(&mnt->mnt_share, &old->mnt_share); - if ((flag & CL_SLAVE) || - ((flag & CL_SHARED_TO_SLAVE) && IS_MNT_SHARED(old))) { + if ((flag & CL_SLAVE) && old->mnt_group_id) { hlist_add_head(&mnt->mnt_slave, &old->mnt_slave_list); mnt->mnt_master = old; } else if (IS_MNT_SLAVE(old)) { @@ -4228,7 +4227,7 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns, /* First pass: copy the tree topology */ copy_flags = CL_COPY_UNBINDABLE | CL_EXPIRE; if (user_ns != ns->user_ns) - copy_flags |= CL_SHARED_TO_SLAVE; + copy_flags |= CL_SLAVE; new = copy_tree(old, old->mnt.mnt_root, copy_flags); if (IS_ERR(new)) { namespace_unlock(); diff --git a/fs/pnode.h b/fs/pnode.h index 507e30e7a420..00ab153e3e9d 100644 --- a/fs/pnode.h +++ b/fs/pnode.h @@ -25,7 +25,6 @@ #define CL_COPY_UNBINDABLE 0x04 #define CL_MAKE_SHARED 0x08 #define CL_PRIVATE 0x10 -#define CL_SHARED_TO_SLAVE 0x20 #define CL_COPY_MNT_NS_FILE 0x40 /* -- cgit v1.2.3 From f6cc2f4e3d304c93b44c80f50430aa40e080cc3c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 25 Jun 2025 15:02:11 -0400 Subject: invent_group_ids(): zero ->mnt_group_id always implies !IS_MNT_SHARED() All places where we call set_mnt_shared() are guaranteed to have non-zero ->mnt_group_id - either by explicit test, or by having done successful invent_group_ids() covering the same mount since we'd grabbed namespace_sem. The opposite combination (non-zero ->mnt_group_id and !IS_MNT_SHARED()) *is* possible - it means that we have allocated group id, but didn't get around to set_mnt_shared() yet; such state is transient - by the time we do namespace_unlock(), we must either do set_mnt_shared() or unroll the group id allocations by cleanup_group_ids(). Signed-off-by: Al Viro --- fs/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index ca36c4a6a143..a75438121417 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2516,7 +2516,7 @@ static int invent_group_ids(struct mount *mnt, bool recurse) struct mount *p; for (p = mnt; p; p = recurse ? next_mnt(p, mnt) : NULL) { - if (!p->mnt_group_id && !IS_MNT_SHARED(p)) { + if (!p->mnt_group_id) { int err = mnt_alloc_group_id(p); if (err) { cleanup_group_ids(mnt, p); -- cgit v1.2.3 From a7cce099450f8fc597a6ac215440666610895fb7 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 25 Jun 2025 15:10:52 -0400 Subject: statmount_mnt_basic(): simplify the logics for group id We are holding namespace_sem shared and we have not done any group id allocations since we grabbed it. Therefore IS_MNT_SHARED(m) is equivalent to non-zero m->mnt_group_id. Signed-off-by: Al Viro --- fs/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/namespace.c b/fs/namespace.c index a75438121417..c549bd39c210 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -5298,7 +5298,7 @@ static void statmount_mnt_basic(struct kstatmount *s) s->sm.mnt_parent_id_old = m->mnt_parent->mnt_id; s->sm.mnt_attr = mnt_to_attr_flags(&m->mnt); s->sm.mnt_propagation = mnt_to_propagation_flags(m); - s->sm.mnt_peer_group = IS_MNT_SHARED(m) ? m->mnt_group_id : 0; + s->sm.mnt_peer_group = m->mnt_group_id; s->sm.mnt_master = IS_MNT_SLAVE(m) ? m->mnt_master->mnt_group_id : 0; } -- cgit v1.2.3