On Mon, Apr 28, 2025 at 10:50:53AM +0200, Christian Brauner wrote:
> I'm not fond of the global variable. I would generally agree with you if
> that were really performance sensitive but this really isn't.
Up to you; propagation calculations *are* hard-serialized (on namespace_sem)
and changing that is too much pain to consider, so I have no problem with
globals in that specific case (note several such in propagate_mnt()
machinery; that was a deliberate decision to avoid shitloads of arguments
that would have to be passed around otherwise), but...
Anyway, minimal fix is to shift clearing the flag, as below.
Longer term I'd rather shift setting and clearing it down into
propagate_mnt() (and dropped check from propagation_would_overmount(),
with corresponding change to can_move_mount_beneath()).
It's really "for the purposes of this mount propagation event treat all
mounts in that namespace as 'new'", so the smaller scope that thing has
the easier it is to reason about...
> I'll have more uses for the flags member very soon as I will make it
> possible to list mounts in anonymous mount namespaces because it
> confuses userspace to no end that they can't list detached mount trees.
>
> So anonymous mount namespaces will simply get a mount namespace id just
> like any other mount namespace and simply be discerned by a flag.
>
> Thanks for going through this. I appreciate it.
>
> The check_mnt() simplification is good though.
FWIW, I've a series of cleanups falling out of audit of struct mount
handling; it's still growing, but I'll post the stable parts for review
tonight or tomorrow...
--------
[PATCH] do_move_mount(): don't leak MNTNS_PROPAGATING on failures
as it is, a failed move_mount(2) from anon namespace breaks
all further propagation into that namespace, including normal
mounts in non-anon namespaces that would otherwise propagate
there.
Fixes: 064fe6e233e8 "mount: handle mount propagation for detached mount trees" v6.15+
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
diff --git a/fs/namespace.c b/fs/namespace.c
index eba4748388b1..8b8348ee5a55 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3714,15 +3714,14 @@ static int do_move_mount(struct path *old_path,
if (err)
goto out;
- if (is_anon_ns(ns))
- ns->mntns_flags &= ~MNTNS_PROPAGATING;
-
/* 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:
+ if (is_anon_ns(ns))
+ ns->mntns_flags &= ~MNTNS_PROPAGATING;
unlock_mount(mp);
if (!err) {
if (attached) {
