From 32f4dc1833d012d07e1e44c0b8662b2cef6e2499 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Sun, 20 Nov 2016 16:34:19 -0500 Subject: [PATCH] Add patches to deal with maliciously crafted file systems. --- add-sanity-checking-in-count_overhead | 36 +++++++++++++ fix-sb-mount-options-processing | 99 +++++++++++++++++++++++++++++++++++ sanity-check-block-and-cluster-size | 64 ++++++++++++++++++++++ series | 4 ++ timestamps | 13 +++-- verify-inodes_per_group-during-mount | 49 +++++++++++++++++ 6 files changed, 261 insertions(+), 4 deletions(-) create mode 100644 add-sanity-checking-in-count_overhead create mode 100644 fix-sb-mount-options-processing create mode 100644 sanity-check-block-and-cluster-size create mode 100644 verify-inodes_per_group-during-mount diff --git a/add-sanity-checking-in-count_overhead b/add-sanity-checking-in-count_overhead new file mode 100644 index 00000000..0d1af2df --- /dev/null +++ b/add-sanity-checking-in-count_overhead @@ -0,0 +1,36 @@ +ext4: add sanity checking to count_overhead() + +The commit "ext4: sanity check the block and cluster size at mount +time" should prevent any problems, but in case the superblock is +modified while the file system is mounted, add an extra safety check +to make sure we won't overrun the allocated buffer. + +Signed-off-by: Theodore Ts'o +Cc: stable@vger.kernel.org +--- + fs/ext4/super.c | 11 ++++++++--- + 1 file changed, 8 insertions(+), 3 deletions(-) + +diff --git a/fs/ext4/super.c b/fs/ext4/super.c +index 689c02df1af4..2d8a49d74f56 100644 +--- a/fs/ext4/super.c ++++ b/fs/ext4/super.c +@@ -3195,10 +3195,15 @@ static int count_overhead(struct super_block *sb, ext4_group_t grp, + ext4_set_bit(s++, buf); + count++; + } +- for (j = ext4_bg_num_gdb(sb, grp); j > 0; j--) { +- ext4_set_bit(EXT4_B2C(sbi, s++), buf); +- count++; ++ j = ext4_bg_num_gdb(sb, grp); ++ if (s + j > EXT4_BLOCKS_PER_GROUP(sb)) { ++ ext4_error(sb, "Invalid number of block group " ++ "descriptor blocks: %d", j); ++ j = EXT4_BLOCKS_PER_GROUP(sb) - s; + } ++ count += j; ++ for (; j > 0; j--) ++ ext4_set_bit(EXT4_B2C(sbi, s++), buf); + } + if (!count) + return 0; diff --git a/fix-sb-mount-options-processing b/fix-sb-mount-options-processing new file mode 100644 index 00000000..46112817 --- /dev/null +++ b/fix-sb-mount-options-processing @@ -0,0 +1,99 @@ +ext4: fix in-superblock mount options processing + +Fix a large number of problems with how we handle mount options in the +superblock. For one, if the string in the superblock is long enough +that it is not null terminated, we could run off the end of the string +and try to interpret superblocks fields as characters. It's unlikely +this will cause a security problem, but it could result in an invalid +parse. Also, parse_options is destructive to the string, so in some +cases if there is a comma-separated string, it would be modified in +the superblock. (Fortunately it only happens on file systems with a +1k block size.) + +Signed-off-by: Theodore Ts'o +Cc: stable@vger.kernel.org +--- + fs/ext4/super.c | 38 +++++++++++++++++++++++--------------- + 1 file changed, 23 insertions(+), 15 deletions(-) + +diff --git a/fs/ext4/super.c b/fs/ext4/super.c +index 0f9ae4ce33d6..404e6f3c1bed 100644 +--- a/fs/ext4/super.c ++++ b/fs/ext4/super.c +@@ -3303,7 +3303,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + char *orig_data = kstrdup(data, GFP_KERNEL); + struct buffer_head *bh; + struct ext4_super_block *es = NULL; +- struct ext4_sb_info *sbi; ++ struct ext4_sb_info *sbi = kzalloc(sizeof(*sbi), GFP_KERNEL); + ext4_fsblk_t block; + ext4_fsblk_t sb_block = get_sb_block(&data); + ext4_fsblk_t logical_sb_block; +@@ -3322,16 +3322,14 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + unsigned int journal_ioprio = DEFAULT_JOURNAL_IOPRIO; + ext4_group_t first_not_zeroed; + +- sbi = kzalloc(sizeof(*sbi), GFP_KERNEL); +- if (!sbi) +- goto out_free_orig; ++ if ((data && !orig_data) || !sbi) ++ goto out_free_base; + + sbi->s_blockgroup_lock = + kzalloc(sizeof(struct blockgroup_lock), GFP_KERNEL); +- if (!sbi->s_blockgroup_lock) { +- kfree(sbi); +- goto out_free_orig; +- } ++ if (!sbi->s_blockgroup_lock) ++ goto out_free_base; ++ + sb->s_fs_info = sbi; + sbi->s_sb = sb; + sbi->s_inode_readahead_blks = EXT4_DEF_INODE_READAHEAD_BLKS; +@@ -3477,11 +3475,19 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + */ + sbi->s_li_wait_mult = EXT4_DEF_LI_WAIT_MULT; + +- if (!parse_options((char *) sbi->s_es->s_mount_opts, sb, +- &journal_devnum, &journal_ioprio, 0)) { +- ext4_msg(sb, KERN_WARNING, +- "failed to parse options in superblock: %s", +- sbi->s_es->s_mount_opts); ++ 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), ++ GFP_KERNEL); ++ if (!s_mount_opts) ++ goto failed_mount; ++ if (!parse_options(s_mount_opts, sb, &journal_devnum, ++ &journal_ioprio, 0)) { ++ ext4_msg(sb, KERN_WARNING, ++ "failed to parse options in superblock: %s", ++ s_mount_opts); ++ } ++ kfree(s_mount_opts); + } + sbi->s_def_mount_opt = sbi->s_mount_opt; + if (!parse_options((char *) data, sb, &journal_devnum, +@@ -4162,7 +4168,9 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + + if (___ratelimit(&ext4_mount_msg_ratelimit, "EXT4-fs mount")) + ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. " +- "Opts: %s%s%s", descr, sbi->s_es->s_mount_opts, ++ "Opts: %.*s%s%s", descr, ++ (int) sizeof(sbi->s_es->s_mount_opts), ++ sbi->s_es->s_mount_opts, + *sbi->s_es->s_mount_opts ? "; " : "", orig_data); + + if (es->s_error_count) +@@ -4241,8 +4249,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + out_fail: + sb->s_fs_info = NULL; + kfree(sbi->s_blockgroup_lock); ++out_free_base: + kfree(sbi); +-out_free_orig: + kfree(orig_data); + return err ? err : ret; + } diff --git a/sanity-check-block-and-cluster-size b/sanity-check-block-and-cluster-size new file mode 100644 index 00000000..f9f64ac2 --- /dev/null +++ b/sanity-check-block-and-cluster-size @@ -0,0 +1,64 @@ +ext4: sanity check the block and cluster size at mount time + +If the block size or cluster size is insane, reject the mount. This +is important for security reasons (although we shouldn't be just +depending on this check). + +Ref: http://www.securityfocus.com/archive/1/539661 +Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1332506 +Reported-by: Borislav Petkov +Reported-by: Nikolay Borisov +Signed-off-by: Theodore Ts'o +Cc: stable@vger.kernel.org +--- + fs/ext4/ext4.h | 1 + + fs/ext4/super.c | 17 ++++++++++++++++- + 2 files changed, 17 insertions(+), 1 deletion(-) + +diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h +index 53d6d463ac4d..bdf1e5ee8642 100644 +--- a/fs/ext4/ext4.h ++++ b/fs/ext4/ext4.h +@@ -235,6 +235,7 @@ struct ext4_io_submit { + #define EXT4_MAX_BLOCK_SIZE 65536 + #define EXT4_MIN_BLOCK_LOG_SIZE 10 + #define EXT4_MAX_BLOCK_LOG_SIZE 16 ++#define EXT4_MAX_CLUSTER_LOG_SIZE 30 + #ifdef __KERNEL__ + # define EXT4_BLOCK_SIZE(s) ((s)->s_blocksize) + #else +diff --git a/fs/ext4/super.c b/fs/ext4/super.c +index 35ccbdc2d64e..0f9ae4ce33d6 100644 +--- a/fs/ext4/super.c ++++ b/fs/ext4/super.c +@@ -3567,7 +3567,15 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + if (blocksize < EXT4_MIN_BLOCK_SIZE || + blocksize > EXT4_MAX_BLOCK_SIZE) { + ext4_msg(sb, KERN_ERR, +- "Unsupported filesystem blocksize %d", blocksize); ++ "Unsupported filesystem blocksize %d (%d log_block_size)", ++ blocksize, le32_to_cpu(es->s_log_block_size)); ++ goto failed_mount; ++ } ++ if (le32_to_cpu(es->s_log_block_size) > ++ (EXT4_MAX_BLOCK_LOG_SIZE - EXT4_MIN_BLOCK_LOG_SIZE)) { ++ ext4_msg(sb, KERN_ERR, ++ "Invalid log block size: %u", ++ le32_to_cpu(es->s_log_block_size)); + goto failed_mount; + } + +@@ -3699,6 +3707,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + "block size (%d)", clustersize, blocksize); + goto failed_mount; + } ++ if (le32_to_cpu(es->s_log_cluster_size) > ++ (EXT4_MAX_CLUSTER_LOG_SIZE - EXT4_MIN_BLOCK_LOG_SIZE)) { ++ ext4_msg(sb, KERN_ERR, ++ "Invalid log cluster size: %u", ++ le32_to_cpu(es->s_log_cluster_size)); ++ goto failed_mount; ++ } + sbi->s_cluster_bits = le32_to_cpu(es->s_log_cluster_size) - + le32_to_cpu(es->s_log_block_size); + sbi->s_clusters_per_group = diff --git a/series b/series index 8636d132..706fd4ed 100644 --- a/series +++ b/series @@ -9,6 +9,10 @@ fix-stack-corruption-with-64k-blocksize use-current_time-for-inode-timestamps allow-inode-expansion-for-nojournal-file-systems remove-parameter-from-ext4_xattr_ibody_set +sanity-check-block-and-cluster-size +fix-sb-mount-options-processing +verify-inodes_per_group-during-mount +add-sanity-checking-in-count_overhead #################################################### # unstable patches diff --git a/timestamps b/timestamps index 2b35b4ba..0bdfbc45 100755 --- a/timestamps +++ b/timestamps @@ -46,12 +46,17 @@ touch -d @1478656998 disable-writeback touch -d @1479092546 allow-ext4_truncate-to-return-an-error touch -d @1479092548 allow-ext4_ext_truncate-to-return-an-error touch -d @1479092549 dont-lock-buffer-head-in-ext4_commit_super-if-holding-spinlock -touch -d @1479092551 stable-boundary touch -d @1479175477 fix-mballoc-breakage-with-64k-block-size touch -d @1479176786 fix-stack-corruption-with-64k-blocksize touch -d @1479177610 use-current_time-for-inode-timestamps touch -d @1479178115 allow-inode-expansion-for-nojournal-file-systems -touch -d @1479178323 timestamps touch -d @1479178608 remove-parameter-from-ext4_xattr_ibody_set -touch -d @1479178624 series -touch -d @1479178639 status +touch -d @1479310377 avoid-lockdep-warning-when-inheriting-encryption-context +touch -d @1479492024 sanity-check-block-and-cluster-size +touch -d @1479493466 fix-sb-mount-options-processing +touch -d @1479493710 verify-inodes_per_group-during-mount +touch -d @1479494158 series +touch -d @1479494267 add-sanity-checking-in-count_overhead +touch -d @1479494327 stable-boundary +touch -d @1479674600 status +touch -d @1479676755 timestamps diff --git a/verify-inodes_per_group-during-mount b/verify-inodes_per_group-during-mount new file mode 100644 index 00000000..257e9c03 --- /dev/null +++ b/verify-inodes_per_group-during-mount @@ -0,0 +1,49 @@ +ext4: use more strict checks for inodes_per_block on mount + +Centralize the checks for inodes_per_block and be more strict to make +sure the inodes_per_block_group can't end up being zero. + +Signed-off-by: Theodore Ts'o +Reviewed-by: Andreas Dilger +Cc: stable@vger.kernel.org +--- + fs/ext4/super.c | 15 ++++++--------- + 1 file changed, 6 insertions(+), 9 deletions(-) + +diff --git a/fs/ext4/super.c b/fs/ext4/super.c +index 404e6f3c1bed..689c02df1af4 100644 +--- a/fs/ext4/super.c ++++ b/fs/ext4/super.c +@@ -3668,12 +3668,16 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + + sbi->s_blocks_per_group = le32_to_cpu(es->s_blocks_per_group); + sbi->s_inodes_per_group = le32_to_cpu(es->s_inodes_per_group); +- if (EXT4_INODE_SIZE(sb) == 0 || EXT4_INODES_PER_GROUP(sb) == 0) +- goto cantfind_ext4; + + sbi->s_inodes_per_block = blocksize / EXT4_INODE_SIZE(sb); + if (sbi->s_inodes_per_block == 0) + goto cantfind_ext4; ++ if (sbi->s_inodes_per_group < sbi->s_inodes_per_block || ++ sbi->s_inodes_per_group > blocksize * 8) { ++ ext4_msg(sb, KERN_ERR, "invalid inodes per group: %lu\n", ++ sbi->s_blocks_per_group); ++ goto failed_mount; ++ } + sbi->s_itb_per_group = sbi->s_inodes_per_group / + sbi->s_inodes_per_block; + sbi->s_desc_per_block = blocksize / EXT4_DESC_SIZE(sb); +@@ -3756,13 +3760,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) + } + sbi->s_cluster_ratio = clustersize / blocksize; + +- if (sbi->s_inodes_per_group > blocksize * 8) { +- ext4_msg(sb, KERN_ERR, +- "#inodes per group too big: %lu", +- sbi->s_inodes_per_group); +- goto failed_mount; +- } +- + /* Do we have standard group size of clustersize * 8 blocks ? */ + if (sbi->s_blocks_per_group == clustersize << 3) + set_opt2(sb, STD_GROUP_SIZE); -- 2.11.4.GIT