On Sat, May 24, 2025 at 12:22:13AM +0100, Al Viro wrote:
> On Fri, May 23, 2025 at 10:37:35PM +0100, Al Viro wrote:
> > On Fri, May 23, 2025 at 10:29:58PM +0100, Al Viro wrote:
> >
> > > This is bogus, IMO. I'm perfectly fine with propagate_one() returning 0
> > > on anon_ns(m->mnt); that would refuse to propagate into *any* anon ns,
> > > but won't screw the propagation between the mounts that are in normal, non-anon
> > > namespaces.
> >
> > IOW, I mean this variant - the only difference from what you've posted is
> > the location of is_anon_ns() test; you do it in IS_MNT_NEW(), this variant
> > has it in propagate_one(). Does the variant below fix regression?
>
> AFAICS, it does suffice to revert the behaviour change on the reproducer
> upthread.
>
> I've replaced the top of viro/vfs.git#fixes with that; commit message there
> is tentative - if nothing else, that's a patch from Christian with slight
> modifications from me. It also needs reported-by, etc.
>
> Said that, could somebody (original reporter) confirm that the variant
> in git.kernel.org:/pub/scm/linux/kernel/git/viro/vfs.git #fixes (head at
> 63e90fcc1807) is OK with them?
>
> And yes, it will need a proper commit message. Christian?
Yes, that looks good to me, thank you!
>
> commit 63e90fcc18072638a62196caae93de66fc6cbc37
> Author: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Date: Fri May 23 19:20:36 2025 -0400
>
> Don't propagate mounts into detached trees
>
> That reverts to behaviour of 6.14 and earlier, with
> fix from "fix IS_MNT_PROPAGATING uses" preserved.
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
>
> diff --git a/fs/mount.h b/fs/mount.h
> index 7aecf2a60472..ad7173037924 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -7,10 +7,6 @@
>
> extern struct list_head notify_list;
>
> -typedef __u32 __bitwise mntns_flags_t;
> -
> -#define MNTNS_PROPAGATING ((__force mntns_flags_t)(1 << 0))
> -
> struct mnt_namespace {
> struct ns_common ns;
> struct mount * root;
> @@ -37,7 +33,6 @@ struct mnt_namespace {
> struct rb_node mnt_ns_tree_node; /* node in the mnt_ns_tree */
> struct list_head mnt_ns_list; /* entry in the sequential list of mounts namespace */
> refcount_t passive; /* number references not pinning @mounts */
> - mntns_flags_t mntns_flags;
> } __randomize_layout;
>
> struct mnt_pcp {
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 1b466c54a357..623cd110076d 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3648,7 +3648,7 @@ static int do_move_mount(struct path *old_path,
> if (!(attached ? check_mnt(old) : is_anon_ns(ns)))
> goto out;
>
> - if (is_anon_ns(ns)) {
> + if (is_anon_ns(ns) && ns == p->mnt_ns) {
> /*
> * Ending up with two files referring to the root of the
> * same anonymous mount namespace would cause an error
> @@ -3656,16 +3656,7 @@ static int do_move_mount(struct path *old_path,
> * twice into the mount tree which would be rejected
> * later. But be explicit about it right here.
> */
> - if ((is_anon_ns(p->mnt_ns) && ns == p->mnt_ns))
> - goto out;
> -
> - /*
> - * If this is an anonymous mount tree ensure that mount
> - * propagation can detect mounts that were just
> - * propagated to the target mount tree so we don't
> - * propagate onto them.
> - */
> - ns->mntns_flags |= MNTNS_PROPAGATING;
> + goto out;
> } else if (is_anon_ns(p->mnt_ns)) {
> /*
> * Don't allow moving an attached mount tree to an
> @@ -3722,8 +3713,6 @@ static int do_move_mount(struct path *old_path,
> if (attached)
> put_mountpoint(old_mp);
> out:
> - if (is_anon_ns(ns))
> - ns->mntns_flags &= ~MNTNS_PROPAGATING;
> unlock_mount(mp);
> if (!err) {
> if (attached) {
> diff --git a/fs/pnode.c b/fs/pnode.c
> index fb77427df39e..ffd429b760d5 100644
> --- a/fs/pnode.c
> +++ b/fs/pnode.c
> @@ -231,8 +231,8 @@ static int propagate_one(struct mount *m, struct mountpoint *dest_mp)
> /* skip if mountpoint isn't visible in m */
> if (!is_subdir(dest_mp->m_dentry, m->mnt.mnt_root))
> return 0;
> - /* skip if m is in the anon_ns we are emptying */
> - if (m->mnt_ns->mntns_flags & MNTNS_PROPAGATING)
> + /* skip if m is in the anon_ns */
> + if (is_anon_ns(m->mnt_ns))
> return 0;
>
> if (peers(m, last_dest)) {
