[RFC][PATCH] switch do_new_mount_fc() to using fc_mount()

[RFC][PATCH] switch do_new_mount_fc() to using fc_mount()

[Al Viro]

[
This is on top of -rc3; if nobody objects, I'll insert that early in series
in viro/vfs.git#work.mount.  It has an impact for LSM folks - ->sb_kern_mount()
would be called without ->s_umount; nothing in-tree cares, but if you have
objections, yell now.
]

Prior to the call of do_new_mount_fc() the caller has just done successful
vfs_get_tree().  Then do_new_mount_fc() does several checks on resulting
superblock, and either does fc_drop_locked() and returns an error or
proceeds to unlock the superblock and call vfs_create_mount().
    
The thing is, there's no reason to delay that unlock + vfs_create_mount() -
the tests do not rely upon the state of ->s_umount and
        fc_drop_locked()
        put_fs_context()
is equivalent to
        unlock ->s_umount
        put_fs_context()

Doing vfs_create_mount() before the checks allows us to move vfs_get_tree()
from caller to do_new_mount_fc() and collapse it with vfs_create_mount()
into an fc_mount() call.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/namespace.c b/fs/namespace.c
index ae6d1312b184..9e1b7319532c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3721,25 +3721,19 @@ static bool mount_too_revealing(const struct super_block *sb, int *new_mnt_flags
 static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
 			   unsigned int mnt_flags)
 {
-	struct vfsmount *mnt;
 	struct pinned_mountpoint mp = {};
 	struct super_block *sb = fc->root->d_sb;
+	struct vfsmount *mnt = fc_mount(fc);
 	int error;
 
+	if (IS_ERR(mnt))
+		return PTR_ERR(mnt);
+
 	error = security_sb_kern_mount(sb);
 	if (!error && mount_too_revealing(sb, &mnt_flags))
 		error = -EPERM;
-
-	if (unlikely(error)) {
-		fc_drop_locked(fc);
-		return error;
-	}
-
-	up_write(&sb->s_umount);
-
-	mnt = vfs_create_mount(fc);
-	if (IS_ERR(mnt))
-		return PTR_ERR(mnt);
+	if (unlikely(error))
+		goto out;
 
 	mnt_warn_timestamp_expiry(mountpoint, mnt);
 
@@ -3747,10 +3741,12 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
 	if (!error) {
 		error = do_add_mount(real_mount(mnt), mp.mp,
 				     mountpoint, mnt_flags);
+		if (!error)
+			mnt = NULL;	// consumed on success
 		unlock_mount(&mp);
 	}
-	if (error < 0)
-		mntput(mnt);
+out:
+	mntput(mnt);
 	return error;
 }
 
@@ -3804,8 +3800,6 @@ static int do_new_mount(struct path *path, const char *fstype, int sb_flags,
 		err = parse_monolithic_mount_data(fc, data);
 	if (!err && !mount_capable(fc))
 		err = -EPERM;
-	if (!err)
-		err = vfs_get_tree(fc);
 	if (!err)
 		err = do_new_mount_fc(fc, path, mnt_flags);
 

Re: [RFC][PATCH] switch do_new_mount_fc() to using fc_mount()

[Paul Moore]

On Tue, Aug 26, 2025 at 2:21 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> [
> This is on top of -rc3; if nobody objects, I'll insert that early in series
> in viro/vfs.git#work.mount.  It has an impact for LSM folks - ->sb_kern_mount()
> would be called without ->s_umount; nothing in-tree cares, but if you have
> objections, yell now.
> ]

Thanks for the heads-up, I'm not aware of anyone currently
posting/working-on patches that would be dependent on this.

> Prior to the call of do_new_mount_fc() the caller has just done successful
> vfs_get_tree().  Then do_new_mount_fc() does several checks on resulting
> superblock, and either does fc_drop_locked() and returns an error or
> proceeds to unlock the superblock and call vfs_create_mount().
>
> The thing is, there's no reason to delay that unlock + vfs_create_mount() -
> the tests do not rely upon the state of ->s_umount and
>         fc_drop_locked()
>         put_fs_context()
> is equivalent to
>         unlock ->s_umount
>         put_fs_context()
>
> Doing vfs_create_mount() before the checks allows us to move vfs_get_tree()
> from caller to do_new_mount_fc() and collapse it with vfs_create_mount()
> into an fc_mount() call.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

-- 
paul-moore.com