On Tue, Aug 26, 2025 at 06:00:44PM +0100, Al Viro wrote:
> FWIW, I wonder if the things would be cleaner if we did security_sb_kern_mount()
> and mount_too_revealing() *after* unlocking the superblock and getting a vfsmount.
> The latter definitely doesn't give a damn about superblock being locked and
> AFAICS neither does the only in-tree instance of ->sb_kern_mount().
> That way we have the real initialization reasonably close to __free() and
> control flow is easier to follow...
Or, better yet, take vfs_get_tree() from do_new_mount() to do_new_mount_fc()
and collapse it with "unlock ->s_umount and call vfs_create_mount()" into
a call of fc_mount(), like the delta below (on top of posted queue, would get reordered
ealier in it and pick the bits of #25 along the way).
Does anyone have objections here? The only real change is that security_sb_kern_mount()
gets called outside of ->s_umount exclusive scope; no in-tree instances care, but I'd
Cc that to LSM list...
diff --git a/fs/namespace.c b/fs/namespace.c
index 63b74d7384fd..6f062dc7f9bf 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3690,22 +3690,18 @@ static int do_new_mount_fc(struct fs_context *fc, const struct path *mountpoint,
unsigned int mnt_flags)
{
struct super_block *sb = fc->root->d_sb;
+ struct vfsmount *mnt __free(mntput) = fc_mount(fc);
int error;
- error = security_sb_kern_mount(sb);
- if (!error && mount_too_revealing(sb, &mnt_flags))
- error = -EPERM;
+ if (IS_ERR(mnt))
+ return PTR_ERR(mnt);
- if (unlikely(error)) {
- fc_drop_locked(fc);
+ error = security_sb_kern_mount(sb);
+ if (unlikely(error))
return error;
- }
- up_write(&sb->s_umount);
-
- struct vfsmount *mnt __free(mntput) = vfs_create_mount(fc);
- if (IS_ERR(mnt))
- return PTR_ERR(mnt);
+ if (mount_too_revealing(sb, &mnt_flags))
+ return -EPERM;
mnt_warn_timestamp_expiry(mountpoint, mnt);
@@ -3767,8 +3763,6 @@ static int do_new_mount(const struct path *path, const char *fstype,
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);
