2022-09-14 15:50:15

by Jan Kara

[permalink] [raw]
Subject: [PATCH 1/2] ext2: Add sanity checks for group and filesystem size

Add sanity check that filesystem size does not exceed the underlying
device size and that group size is big enough so that metadata can fit
into it. This avoid trying to mount some crafted filesystems with
extremely large group counts.

Reported-by: [email protected]
Signed-off-by: Jan Kara <[email protected]>
---
fs/ext2/super.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 252c742379cf..c94955b6701c 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1052,6 +1052,13 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
sbi->s_blocks_per_group);
goto failed_mount;
}
+ /* At least inode table, bitmaps, and sb have to fit in one group */
+ if (sbi->s_blocks_per_group <= sbi->s_inodes_per_group + 3) {
+ ext2_msg(sb, KERN_ERR,
+ "error: #blocks per group smaller than metadata size: %lu <= %lu",
+ sbi->s_blocks_per_group, sbi->s_inodes_per_group + 3);
+ goto failed_mount;
+ }
if (sbi->s_frags_per_group > sb->s_blocksize * 8) {
ext2_msg(sb, KERN_ERR,
"error: #fragments per group too big: %lu",
@@ -1065,9 +1072,14 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
sbi->s_inodes_per_group);
goto failed_mount;
}
+ if (sb_bdev_nr_blocks(sb) < le32_to_cpu(es->s_blocks_count)) {
+ ext2_msg(sb, KERN_ERR,
+ "bad geometry: block count %u exceeds size of device (%u blocks)",
+ le32_to_cpu(es->s_blocks_count),
+ (unsigned)sb_bdev_nr_blocks(sb));
+ goto failed_mount;
+ }

- if (EXT2_BLOCKS_PER_GROUP(sb) == 0)
- goto cantfind_ext2;
sbi->s_groups_count = ((le32_to_cpu(es->s_blocks_count) -
le32_to_cpu(es->s_first_data_block) - 1)
/ EXT2_BLOCKS_PER_GROUP(sb)) + 1;
--
2.35.3


2022-09-21 06:50:14

by kernel test robot

[permalink] [raw]
Subject: [ext2] 16339bc257: xfstests.generic.226.fail


Greeting,

FYI, we noticed the following commit (built with gcc-11):

commit: 16339bc25762eb9ce9b75035ed5beea2693db094 ("[PATCH 1/2] ext2: Add sanity checks for group and filesystem size")
url: https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/ext2-Handle-corrupted-sb-better/20220914-234926
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 3245cb65fd91cd514801bf91f5a3066d562f0ac4
patch link: https://lore.kernel.org/linux-ext4/[email protected]

in testcase: xfstests
version: xfstests-x86_64-c1144bf-1_20220906
with following parameters:

disk: 4HDD
fs: ext2
test: generic-group-11

test-description: xfstests is a regression test suite for xfs and other files ystems.
test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git


on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz (Ivy Bridge) with 8G memory

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):




If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]


generic/226 [failed, exit status 1]- output mismatch (see /lkp/benchmarks/xfstests/results//generic/226.out.bad)
--- tests/generic/226.out 2022-09-06 02:42:28.000000000 +0000
+++ /lkp/benchmarks/xfstests/results//generic/226.out.bad 2022-09-20 17:31:28.750779050 +0000
@@ -1,6 +1,5 @@
QA output created by 226
--> mkfs 256m filesystem
---> 16 buffered 64m writes in a loop
-1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
---> 16 direct 64m writes in a loop
-1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
+mount: /fs/scratch: wrong fs type, bad option, bad superblock on /dev/sda4, missing codepage or helper program, or other error.
...
(Run 'diff -u /lkp/benchmarks/xfstests/tests/generic/226.out /lkp/benchmarks/xfstests/results//generic/226.out.bad' to see the entire diff)


To reproduce:

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.



--
0-DAY CI Kernel Test Service
https://01.org/lkp



Attachments:
(No filename) (2.49 kB)
config-6.0.0-rc5-00026-g16339bc25762 (169.97 kB)
job-script (6.17 kB)
dmesg.xz (1.75 kB)
xfstests (3.55 kB)
job.yaml (5.03 kB)
reproduce (968.00 B)
Download all attachments

2022-09-21 08:05:16

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext2: Add sanity checks for group and filesystem size

On Wed 14-09-22 17:47:22, Jan Kara wrote:
> Add sanity check that filesystem size does not exceed the underlying
> device size and that group size is big enough so that metadata can fit
> into it. This avoid trying to mount some crafted filesystems with
> extremely large group counts.
>
> Reported-by: [email protected]
> Signed-off-by: Jan Kara <[email protected]>
...
> + /* At least inode table, bitmaps, and sb have to fit in one group */
> + if (sbi->s_blocks_per_group <= sbi->s_inodes_per_group + 3) {

Indeed this should have been comparing against number of inode *table
blocks*, not number of inodes... I've fixed the patch locally, the result
is attached for reference.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR


Attachments:
(No filename) (792.00 B)
0001-ext2-Add-sanity-checks-for-group-and-filesystem-size.patch (2.12 kB)
Download all attachments