ext4: validate the debug_want_extra_isize mount option at parse time

Instead of setting s_want_extra_size and then making sure that it is a
valid value afterwards, validate the field before we set it.  This
avoids races and other problems when remounting the file system.

Link: https://lore.kernel.org/r/20191215063020.GA11512@mit.edu
Cc: stable@kernel.org
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reported-and-tested-by: syzbot+4a39a025912b265cacef@syzkaller.appspotmail.com

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index b205112ca05113dc6f86f2b9a7c0ef609e7d6303..46b6d5b150ac9f665341a11968c292f24e32e249 100644 (file)
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1886,6 +1886,13 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
                }
                sbi->s_commit_interval = HZ * arg;
        } else if (token == Opt_debug_want_extra_isize) {
+               if ((arg & 1) ||
+                   (arg < 4) ||
+                   (arg > (sbi->s_inode_size - EXT4_GOOD_OLD_INODE_SIZE))) {
+                       ext4_msg(sb, KERN_ERR,
+                                "Invalid want_extra_isize %d", arg);
+                       return -1;
+               }
                sbi->s_want_extra_isize = arg;
        } else if (token == Opt_max_batch_time) {
                sbi->s_max_batch_time = arg;
@@ -3540,40 +3547,6 @@ int ext4_calculate_overhead(struct super_block *sb)
        return 0;
 }
 
-static void ext4_clamp_want_extra_isize(struct super_block *sb)
-{
-       struct ext4_sb_info *sbi = EXT4_SB(sb);
-       struct ext4_super_block *es = sbi->s_es;
-       unsigned def_extra_isize = sizeof(struct ext4_inode) -
-                                               EXT4_GOOD_OLD_INODE_SIZE;
-
-       if (sbi->s_inode_size == EXT4_GOOD_OLD_INODE_SIZE) {
-               sbi->s_want_extra_isize = 0;
-               return;
-       }
-       if (sbi->s_want_extra_isize < 4) {
-               sbi->s_want_extra_isize = def_extra_isize;
-               if (ext4_has_feature_extra_isize(sb)) {
-                       if (sbi->s_want_extra_isize <
-                           le16_to_cpu(es->s_want_extra_isize))
-                               sbi->s_want_extra_isize =
-                                       le16_to_cpu(es->s_want_extra_isize);
-                       if (sbi->s_want_extra_isize <
-                           le16_to_cpu(es->s_min_extra_isize))
-                               sbi->s_want_extra_isize =
-                                       le16_to_cpu(es->s_min_extra_isize);
-               }
-       }
-       /* Check if enough inode space is available */
-       if ((sbi->s_want_extra_isize > sbi->s_inode_size) ||
-           (EXT4_GOOD_OLD_INODE_SIZE + sbi->s_want_extra_isize >
-                                                       sbi->s_inode_size)) {
-               sbi->s_want_extra_isize = def_extra_isize;
-               ext4_msg(sb, KERN_INFO,
-                        "required extra inode space not available");
-       }
-}
-
 static void ext4_set_resv_clusters(struct super_block *sb)
 {
        ext4_fsblk_t resv_clusters;
@@ -3781,6 +3754,68 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
         */
        sbi->s_li_wait_mult = EXT4_DEF_LI_WAIT_MULT;
 
+       if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) {
+               sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE;
+               sbi->s_first_ino = EXT4_GOOD_OLD_FIRST_INO;
+       } else {
+               sbi->s_inode_size = le16_to_cpu(es->s_inode_size);
+               sbi->s_first_ino = le32_to_cpu(es->s_first_ino);
+               if (sbi->s_first_ino < EXT4_GOOD_OLD_FIRST_INO) {
+                       ext4_msg(sb, KERN_ERR, "invalid first ino: %u",
+                                sbi->s_first_ino);
+                       goto failed_mount;
+               }
+               if ((sbi->s_inode_size < EXT4_GOOD_OLD_INODE_SIZE) ||
+                   (!is_power_of_2(sbi->s_inode_size)) ||
+                   (sbi->s_inode_size > blocksize)) {
+                       ext4_msg(sb, KERN_ERR,
+                              "unsupported inode size: %d",
+                              sbi->s_inode_size);
+                       goto failed_mount;
+               }
+               /*
+                * i_atime_extra is the last extra field available for
+                * [acm]times in struct ext4_inode. Checking for that
+                * field should suffice to ensure we have extra space
+                * for all three.
+                */
+               if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) +
+                       sizeof(((struct ext4_inode *)0)->i_atime_extra)) {
+                       sb->s_time_gran = 1;
+                       sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
+               } else {
+                       sb->s_time_gran = NSEC_PER_SEC;
+                       sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX;
+               }
+               sb->s_time_min = EXT4_TIMESTAMP_MIN;
+       }
+       if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE) {
+               sbi->s_want_extra_isize = sizeof(struct ext4_inode) -
+                       EXT4_GOOD_OLD_INODE_SIZE;
+               if (ext4_has_feature_extra_isize(sb)) {
+                       unsigned v, max = (sbi->s_inode_size -
+                                          EXT4_GOOD_OLD_INODE_SIZE);
+
+                       v = le16_to_cpu(es->s_want_extra_isize);
+                       if (v > max) {
+                               ext4_msg(sb, KERN_ERR,
+                                        "bad s_want_extra_isize: %d", v);
+                               goto failed_mount;
+                       }
+                       if (sbi->s_want_extra_isize < v)
+                               sbi->s_want_extra_isize = v;
+
+                       v = le16_to_cpu(es->s_min_extra_isize);
+                       if (v > max) {
+                               ext4_msg(sb, KERN_ERR,
+                                        "bad s_min_extra_isize: %d", v);
+                               goto failed_mount;
+                       }
+                       if (sbi->s_want_extra_isize < v)
+                               sbi->s_want_extra_isize = v;
+               }
+       }
+
        if (sbi->s_es->s_mount_opts[0]) {
                char *s_mount_opts = kstrndup(sbi->s_es->s_mount_opts,
                                              sizeof(sbi->s_es->s_mount_opts),
@@ -4019,42 +4054,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
                                                      has_huge_files);
        sb->s_maxbytes = ext4_max_size(sb->s_blocksize_bits, has_huge_files);
 
-       if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) {
-               sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE;
-               sbi->s_first_ino = EXT4_GOOD_OLD_FIRST_INO;
-       } else {
-               sbi->s_inode_size = le16_to_cpu(es->s_inode_size);
-               sbi->s_first_ino = le32_to_cpu(es->s_first_ino);
-               if (sbi->s_first_ino < EXT4_GOOD_OLD_FIRST_INO) {
-                       ext4_msg(sb, KERN_ERR, "invalid first ino: %u",
-                                sbi->s_first_ino);
-                       goto failed_mount;
-               }
-               if ((sbi->s_inode_size < EXT4_GOOD_OLD_INODE_SIZE) ||
-                   (!is_power_of_2(sbi->s_inode_size)) ||
-                   (sbi->s_inode_size > blocksize)) {
-                       ext4_msg(sb, KERN_ERR,
-                              "unsupported inode size: %d",
-                              sbi->s_inode_size);
-                       goto failed_mount;
-               }
-               /*
-                * i_atime_extra is the last extra field available for [acm]times in
-                * struct ext4_inode. Checking for that field should suffice to ensure
-                * we have extra space for all three.
-                */
-               if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) +
-                       sizeof(((struct ext4_inode *)0)->i_atime_extra)) {
-                       sb->s_time_gran = 1;
-                       sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
-               } else {
-                       sb->s_time_gran = NSEC_PER_SEC;
-                       sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX;
-               }
-
-               sb->s_time_min = EXT4_TIMESTAMP_MIN;
-       }
-
        sbi->s_desc_size = le16_to_cpu(es->s_desc_size);
        if (ext4_has_feature_64bit(sb)) {
                if (sbi->s_desc_size < EXT4_MIN_DESC_SIZE_64BIT ||
@@ -4503,8 +4502,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
        } else if (ret)
                goto failed_mount4a;
 
-       ext4_clamp_want_extra_isize(sb);
-
        ext4_set_resv_clusters(sb);
 
        err = ext4_setup_system_zone(sb);
@@ -5292,8 +5289,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
                goto restore_opts;
        }
 
-       ext4_clamp_want_extra_isize(sb);
-
        if ((old_opts.s_mount_opt & EXT4_MOUNT_JOURNAL_CHECKSUM) ^
            test_opt(sb, JOURNAL_CHECKSUM)) {
                ext4_msg(sb, KERN_ERR, "changing journal_checksum "