> On Nov 28, 2016, at 10:57 PM, Eryu Guan <guaneryu@xxxxxxxxx> wrote: > > 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 <ralf@xxxxxxx> > Signed-off-by: Eryu Guan <guaneryu@xxxxxxxxx> Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx> > --- > > 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 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail