From: Eryu Guan Subject: [PATCH] ext4: validate s_first_meta_bg at mount time Date: Tue, 29 Nov 2016 13:57:17 +0800 Message-ID: <20161129055717.4154-1-guaneryu@gmail.com> Cc: tytso@mit.edu, Eryu Guan To: linux-ext4@vger.kernel.org Return-path: Received: from mail-pg0-f68.google.com ([74.125.83.68]:35254 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753135AbcK2GaJ (ORCPT ); Tue, 29 Nov 2016 01:30:09 -0500 Received: by mail-pg0-f68.google.com with SMTP id p66so15386603pga.2 for ; Mon, 28 Nov 2016 22:30:09 -0800 (PST) Sender: linux-ext4-owner@vger.kernel.org List-ID: Ralf Spenneberg reported that he hit a kernel crash when mounting a modified ext4 image. And it turns out that kernel crashed when calculating fs overhead (ext4_calculate_overhead()), this is because the image has very large s_first_meta_bg (debug code shows it's 842150400), and ext4 overruns the memory in count_overhead() when setting bitmap buffer, which is PAGE_SIZE. ext4_calculate_overhead(): buf = get_zeroed_page(GFP_NOFS); <=== PAGE_SIZE buffer blks = count_overhead(sb, i, buf); count_overhead(): for (j = ext4_bg_num_gdb(sb, grp); j > 0; j--) { <=== j = 842150400 ext4_set_bit(EXT4_B2C(sbi, s++), buf); <=== buffer overrun count++; } This can be reproduced easily for me by this script: #!/bin/bash rm -f fs.img mkdir -p /mnt/ext4 fallocate -l 16M fs.img mke2fs -t ext4 -O bigalloc,meta_bg,^resize_inode -F fs.img debugfs -w -R "ssv first_meta_bg 842150400" fs.img mount -o loop fs.img /mnt/ext4 Fix it by validating s_first_meta_bg first at mount time, and refusing to mount if its value exceeds the largest possible meta_bg number. Reported-by: Ralf Spenneberg Signed-off-by: Eryu Guan --- In e2fsprogs, we avoided a similar buffer overrun: f66e6ce libext2fs: avoid buffer overflow if s_first_meta_bg is too big and e2fsck could detect & fix large s_first_meta_bg: 7a4352d e2fsck: fix file systems with an overly large s_first_meta_bg But I suspect that there's an off-by-one bug in e2fsck code, shouldn't the upper boundary of s_first_meta_bg be (fs->desc_blocks - 1)? Something like: e2fsck/super.c:643 if (ext2fs_has_feature_meta_bg(fs->super) && - (fs->super->s_first_meta_bg > fs->desc_blocks)) { - pctx.group = fs->desc_blocks; + (fs->super->s_first_meta_bg >= fs->desc_blocks)) { + pctx.group = fs->desc_blocks - 1; pctx.num = fs->super->s_first_meta_bg; fs/ext4/super.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 52b0530..8f46a07 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3814,6 +3814,15 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) (EXT4_MAX_BLOCK_FILE_PHYS / EXT4_BLOCKS_PER_GROUP(sb))); db_count = (sbi->s_groups_count + EXT4_DESC_PER_BLOCK(sb) - 1) / EXT4_DESC_PER_BLOCK(sb); + if (ext4_has_feature_meta_bg(sb)) { + if (le32_to_cpu(es->s_first_meta_bg) >= db_count) { + ext4_msg(sb, KERN_WARNING, + "first meta block group too large: %u " + "(group descriptor block count %u)", + le32_to_cpu(es->s_first_meta_bg), db_count); + goto failed_mount; + } + } sbi->s_group_desc = ext4_kvmalloc(db_count * sizeof(struct buffer_head *), GFP_KERNEL); -- 2.9.3