From:   =?UTF-8?q?Michal=20Koutn=C3=BD?= <mkoutny@suse.com>
To:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Alexander Viro <viro@zeniv.linux.org.uk>,
        Petr Mladek <pmladek@suse.com>,
        Petr Pavlu <petr.pavlu@suse.com>
Subject: [RFC PATCH] vfs: Delay root FS switch after UMH completion
Date:   Mon,  6 Feb 2023 18:10:32 +0100
Message-Id: <20230206171032.12801-1-mkoutny@suse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

We want to make sure no UMHs started with an old root survive into the
world with the new root (they may fail when it is not expected).
Therefore, insert a wait for existing UMHs termination (this assumes UMH
runtime is finite).

A motivation are asynchronous module loads that start in initrd and they
may be (un)intentionally terminated by a userspace cleanup during rootfs
transition.

This is also inspired by an ancient problem [1].

This is just a rough idea, only superficially tested, no broader context
(VFS locking et al) is considered.

[1] https://kernelnewbies.org/KernelProjects/usermode-helper-enhancements#Filesystem_suspend

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---

I'm not amused by this patch. I'm sending it to get some NAck reasons
(besides indefinite UMH lifetime) and get it off my head. Thanks.

 fs/namespace.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index ab467ee58341..48cb658ae10c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2931,6 +2931,7 @@ static int do_move_mount(struct path *old_path, struct path *new_path)
 	struct mount *old;
 	struct mount *parent;
 	struct mountpoint *mp, *old_mp;
+	struct path ns_root;
 	int err;
 	bool attached;
 
@@ -2985,6 +2986,14 @@ static int do_move_mount(struct path *old_path, struct path *new_path)
 		if (p == old)
 			goto out;
 
+	ns_root.mnt = &current->nsproxy->mnt_ns->root->mnt;
+	ns_root.dentry = ns_root.mnt->mnt_root;
+	path_get(&ns_root);
+	/* See argument in pivot_root() */
+	if (path_equal(new_path, &ns_root))
+		usermodehelper_disable();
+
+
 	err = attach_recursive_mnt(old, real_mount(new_path->mnt), mp,
 				   attached);
 	if (err)
@@ -2996,6 +3005,9 @@ static int do_move_mount(struct path *old_path, struct path *new_path)
 	if (attached)
 		put_mountpoint(old_mp);
 out:
+	if (path_equal(new_path, &ns_root))
+		usermodehelper_enable();
+
 	unlock_mount(mp);
 	if (!err) {
 		if (attached)
@@ -3997,6 +4009,8 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
 		goto out2;
 
 	get_fs_root(current->fs, &root);
+	/* UMHs started from old root should finish before we switch root under */
+	usermodehelper_disable(); // XXX error handling
 	old_mp = lock_mount(&old);
 	error = PTR_ERR(old_mp);
 	if (IS_ERR(old_mp))
@@ -4058,6 +4072,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
 	error = 0;
 out4:
 	unlock_mount(old_mp);
+	usermodehelper_enable();
 	if (!error)
 		mntput_no_expire(ex_parent);
 out3:
-- 
2.39.1