From: Al Viro <viro@zeniv.linux.org.uk>
To: Christian Brauner <brauner@kernel.org>
Cc: Allison Karlitskaya <lis@redhat.com>, linux-fsdevel@vger.kernel.org
Subject: Re: Apparent mount behaviour change in 6.15
Date: Sat, 24 May 2025 00:22:13 +0100 [thread overview]
Message-ID: <20250523232213.GL2023217@ZenIV> (raw)
In-Reply-To: <20250523213735.GK2023217@ZenIV>
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?
commit 63e90fcc18072638a62196caae93de66fc6cbc37
Author: Al Viro <viro@zeniv.linux.org.uk>
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@zeniv.linux.org.uk>
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)) {
next prev parent reply other threads:[~2025-05-23 23:22 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-15 4:06 Apparent mount behaviour change in 6.15 Allison Karlitskaya
2025-05-15 11:25 ` Christian Brauner
2025-05-23 6:32 ` Al Viro
2025-05-23 8:41 ` Christian Brauner
2025-05-23 21:29 ` Al Viro
2025-05-23 21:37 ` Al Viro
2025-05-23 23:22 ` Al Viro [this message]
2025-05-26 4:47 ` Christian Brauner
2025-05-26 21:32 ` Al Viro
2025-05-26 7:18 ` Allison Karlitskaya
2025-05-26 8:00 ` Allison Karlitskaya
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250523232213.GL2023217@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=brauner@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=lis@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).