vfs_get_tree(): evict the call of security_sb_kern_mount()

Right now vfs_get_tree() calls security_sb_kern_mount() (i.e.
mount MAC) unless it gets MS_KERNMOUNT or MS_SUBMOUNT in flags.
Doing it that way is both clumsy and imprecise.

Consider the callers' tree of vfs_get_tree():
vfs_get_tree()
        <- do_new_mount()
	<- vfs_kern_mount()
		<- simple_pin_fs()
		<- vfs_submount()
		<- kern_mount_data()
		<- init_mount_tree()
		<- btrfs_mount()
			<- vfs_get_tree()
		<- nfs_do_root_mount()
			<- nfs4_try_mount()
				<- nfs_fs_mount()
					<- vfs_get_tree()
			<- nfs4_referral_mount()

do_new_mount() always does need MAC (we are guaranteed that neither
MS_KERNMOUNT nor MS_SUBMOUNT will be passed there).

simple_pin_fs(), vfs_submount() and kern_mount_data() pass explicit
flags inhibiting that check.  So does nfs4_referral_mount() (the
flags there are ulimately coming from vfs_submount()).

init_mount_tree() is called too early for anything LSM-related; it
doesn't matter whether we attempt those checks, they'll do nothing.

Finally, in case of btrfs_mount() and nfs_fs_mount(), doing MAC
is pointless - either the caller will do it, or the flags are
such that we wouldn't have done it either.

In other words, the one and only case when we want that check
done is when we are called from do_new_mount(), and there we
want it unconditionally.

So let's simply move it there.  The superblock is still locked,
so nobody is going to get access to it (via ustat(2), etc.)
until we get a chance to apply the checks - we are free to
move them to any point up to where we drop ->s_umount (in
do_new_mount_fc()).

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/fs_context.c b/fs/fs_context.c
index 4294091b689dabc1c6495ee6d1d58dfdb1103754..857cd46a687b786b57ee3a025fe198f4a4929fe8 100644 (file)
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -90,6 +90,14 @@ struct fs_context *fs_context_for_mount(struct file_system_type *fs_type,
 }
 EXPORT_SYMBOL(fs_context_for_mount);
 
+void fc_drop_locked(struct fs_context *fc)
+{
+       struct super_block *sb = fc->root->d_sb;
+       dput(fc->root);
+       fc->root = NULL;
+       deactivate_locked_super(sb);
+}
+
 static void legacy_fs_context_free(struct fs_context *fc);
 /**
  * put_fs_context - Dispose of a superblock configuration context.

diff --git a/fs/internal.h b/fs/internal.h
index f85c3212d25dc28ac8492715152effbc967b179b..6af26d897034b9834b7d6bf94d41ceeb27b78a4e 100644 (file)
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -57,6 +57,7 @@ extern void __init chrdev_init(void);
  */
 extern int legacy_get_tree(struct fs_context *fc);
 extern int parse_monolithic_mount_data(struct fs_context *, void *);
+extern void fc_drop_locked(struct fs_context *);
 
 /*
  * namei.c

diff --git a/fs/namespace.c b/fs/namespace.c
index f629e1c7f3cc0a1f7eab278a1092e283c008d42d..750500c6c33d19add8ae7429285bf398a97a3162 100644 (file)
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2536,11 +2536,13 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
        struct super_block *sb = fc->root->d_sb;
        int error;
 
-       if (mount_too_revealing(sb, &mnt_flags)) {
-               dput(fc->root);
-               fc->root = NULL;
-               deactivate_locked_super(sb);
-               return -EPERM;
+       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);

diff --git a/fs/super.c b/fs/super.c
index b91b6df05b67c347c2e7d5dbd890bb3b0e922b7c..11e2a6cb3bafa408a126bcb99f977a03d4a4a784 100644 (file)
--- a/fs/super.c
+++ b/fs/super.c
@@ -1277,13 +1277,9 @@ int vfs_get_tree(struct fs_context *fc)
        sb->s_flags |= SB_BORN;
 
        error = security_sb_set_mnt_opts(sb, fc->security, 0, NULL);
-       if (error)
-               goto out_sb;
-
-       if (!(fc->sb_flags & (MS_KERNMOUNT|MS_SUBMOUNT))) {
-               error = security_sb_kern_mount(sb);
-               if (error)
-                       goto out_sb;
+       if (unlikely(error)) {
+               fc_drop_locked(fc);
+               return error;
        }
 
        /*
@@ -1296,11 +1292,6 @@ int vfs_get_tree(struct fs_context *fc)
                "negative value (%lld)\n", fc->fs_type->name, sb->s_maxbytes);
 
        return 0;
-out_sb:
-       dput(fc->root);
-       fc->root = NULL;
-       deactivate_locked_super(sb);
-       return error;
 }
 EXPORT_SYMBOL(vfs_get_tree);