All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Christian Brauner <brauner@kernel.org>
Cc: linux-fsdevel@vger.kernel.org, Miklos Szeredi <miklos@szeredi.hu>,
	Jan Kara <jack@suse.cz>, Allison Karlitskaya <lis@redhat.com>
Subject: Re: [PATCH 1/2] mount: fix detached mount regression
Date: Fri, 6 Jun 2025 08:01:27 +0100	[thread overview]
Message-ID: <20250606070127.GU299672@ZenIV> (raw)
In-Reply-To: <20250606051428.GT299672@ZenIV>

	Folks, could you check the following, on top of viro/vfs.git#fixes?

do_move_mount(): split the checks in subtree-of-our-ns and entire-anon cases

... and fix the breakage in anon-to-anon case.  There are two cases
acceptable for do_move_mount() and mixing checks for those is making
things hard to follow.

One case is move of a subtree in caller's namespace.
	* source and destination must be in caller's namespace
	* source must be detachable from parent
Another is moving the entire anon namespace elsewhere
	* source must be the root of anon namespace
	* target must either in caller's namespace or in a suitable
	  anon namespace (see may_use_mount() for details).
	* target must not be in the same namespace as source.

It's really easier to follow if tests are *not* mixed together...

[[
This should be equivalent to what Christian has posted last night;
please test on whatever tests you've got.
]]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 854099aafed5..59197109e6b9 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3656,37 +3656,29 @@ static int do_move_mount(struct path *old_path,
 	ns = old->mnt_ns;
 
 	err = -EINVAL;
-	if (!may_use_mount(p))
-		goto out;
-
 	/* The thing moved must be mounted... */
 	if (!is_mounted(&old->mnt))
 		goto out;
 
 	/* ... and either ours or the root of anon namespace */
-	if (!(attached ? check_mnt(old) : is_anon_ns(ns)))
-		goto out;
-
-	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
-		 * as this would mean trying to move the same mount
-		 * twice into the mount tree which would be rejected
-		 * later. But be explicit about it right here.
-		 */
-		goto out;
-	} else if (is_anon_ns(p->mnt_ns)) {
-		/*
-		 * Don't allow moving an attached mount tree to an
-		 * anonymous mount tree.
-		 */
-		goto out;
+	if (check_mnt(old)) {
+		/* should be detachable from parent */
+		if (!mnt_has_parent(old) || IS_MNT_LOCKED(old))
+			goto out;
+		/* target should be ours */
+		if (!check_mnt(p))
+			goto out;
+	} else {
+		if (!is_anon_ns(ns) || mnt_has_parent(old))
+			goto out;
+		/* not into the same anon ns - bail early in that case */
+		if (ns == p->mnt_ns)
+			goto out;
+		/* target should be ours or in an acceptable anon ns */
+		if (!may_use_mount(p))
+			goto out;
 	}
 
-	if (old->mnt.mnt_flags & MNT_LOCKED)
-		goto out;
-
 	if (!path_mounted(old_path))
 		goto out;
 

  reply	other threads:[~2025-06-06  7:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-05 12:50 [PATCH 0/2] mount: fix detached mount regression Christian Brauner
2025-06-05 12:50 ` [PATCH 1/2] " Christian Brauner
2025-06-06  4:54   ` Al Viro
2025-06-06  5:14     ` Al Viro
2025-06-06  7:01       ` Al Viro [this message]
2025-06-06  7:58         ` Christian Brauner
2025-06-06 17:45           ` Al Viro
2025-06-07  5:20             ` Al Viro
2025-06-11  9:36               ` Christian Brauner
2025-06-11 17:29                 ` Al Viro
2025-06-12 12:16                   ` Christian Brauner
2025-06-05 12:50 ` [PATCH 2/2] selftests/mount_setattr: adapt detached mount propagation test Christian Brauner

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=20250606070127.GU299672@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=lis@redhat.com \
    --cc=miklos@szeredi.hu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.