[PATCH] ext4: validate s_first_meta_bg at mount time

[PATCH] ext4: validate s_first_meta_bg at mount time

[Eryu Guan]

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@os-t.de>
Signed-off-by: Eryu Guan <guaneryu@gmail.com>
---

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

Re: [PATCH] ext4: validate s_first_meta_bg at mount time

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 3373 bytes --]


> On Nov 28, 2016, at 10:57 PM, Eryu Guan <guaneryu@gmail.com> 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@os-t.de>
> Signed-off-by: Eryu Guan <guaneryu@gmail.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> 
> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] ext4: validate s_first_meta_bg at mount time

[Theodore Ts'o]

On Tue, Nov 29, 2016 at 01:57:17PM +0800, Eryu Guan 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.
>...
> 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@os-t.de>
> Signed-off-by: Eryu Guan <guaneryu@gmail.com>

Thanks, applied.  And yes, I do believe you are right about e2fsck
having an off-by-one error.  Will fix.

					- Ted