[PATCH 1/6] libext2fs: avoid buffer overflow if s_first_meta_bg is too big

[PATCH 1/6] libext2fs: avoid buffer overflow if s_first_meta_bg is too big

[Theodore Ts'o]

If s_first_meta_bg is greater than the number block group descriptor
blocks, then reading or writing the block group descriptors will end
up overruning the memory buffer allocated for the descriptors.  Fix
this by limiting first_meta_bg to no more than fs->desc_blocks.  This
doesn't correct the bad s_first_meta_bg value, but it avoids causing
the e2fsprogs userspace programs from potentially crashing.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 lib/ext2fs/closefs.c | 6 ++++--
 lib/ext2fs/openfs.c  | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c
index 4599eef..1f99113 100644
--- a/lib/ext2fs/closefs.c
+++ b/lib/ext2fs/closefs.c
@@ -344,9 +344,11 @@ errcode_t ext2fs_flush2(ext2_filsys fs, int flags)
 	 * superblocks and group descriptors.
 	 */
 	group_ptr = (char *) group_shadow;
-	if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
+	if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG) {
 		old_desc_blocks = fs->super->s_first_meta_bg;
-	else
+		if (old_desc_blocks > fs->super->s_first_meta_bg)
+			old_desc_blocks = fs->desc_blocks;
+	} else
 		old_desc_blocks = fs->desc_blocks;
 
 	ext2fs_numeric_progress_init(fs, &progress, NULL,
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index a1a3517..ba501e6 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -378,9 +378,11 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
 #ifdef WORDS_BIGENDIAN
 	groups_per_block = EXT2_DESC_PER_BLOCK(fs->super);
 #endif
-	if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
+	if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG) {
 		first_meta_bg = fs->super->s_first_meta_bg;
-	else
+		if (first_meta_bg > fs->desc_blocks)
+			first_meta_bg = fs->desc_blocks;
+	} else
 		first_meta_bg = fs->desc_blocks;
 	if (first_meta_bg) {
 		retval = io_channel_read_blk(fs->io, group_block +
-- 
2.0.0

[PATCH 2/6] e2fsck: fix file systems with an overly large s_first_meta_bg

[Theodore Ts'o]

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 e2fsck/problem.c |  5 +++++
 e2fsck/problem.h |  3 +++
 e2fsck/super.c   | 12 ++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 6d9b1af..57c2e39 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -440,6 +440,11 @@ static struct e2fsck_problem problem_table[] = {
 	  N_("@S 64bit filesystems needs extents to access the whole disk.  "),
 	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK},
 
+	/* The first_meta_bg is too big */
+	{ PR_0_FIRST_META_BG_TOO_BIG,
+	  N_("First_meta_bg is too big.  (%N, max value %g).  "),
+	  PROMPT_CLEAR, 0 },
+
 	/* Pass 1 errors */
 
 	/* Pass 1: Checking inodes, blocks, and sizes */
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index d673a4e..3426a22 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -252,6 +252,9 @@ struct problem_context {
 /* 64bit is set but extents are not set. */
 #define PR_0_64BIT_WITHOUT_EXTENTS		0x000048
 
+/* The first_meta_bg is too big */
+#define PR_0_FIRST_META_BG_TOO_BIG		0x000049
+
 /*
  * Pass 1 errors
  */
diff --git a/e2fsck/super.c b/e2fsck/super.c
index 8d468e6..768316a 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -591,6 +591,18 @@ void check_super_block(e2fsck_t ctx)
 		ext2fs_mark_super_dirty(fs);
 	}
 
+	if ((fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG) &&
+	    (fs->super->s_first_meta_bg > fs->desc_blocks)) {
+		pctx.group = fs->desc_blocks;
+		pctx.num = fs->super->s_first_meta_bg;
+		if (fix_problem(ctx, PR_0_FIRST_META_BG_TOO_BIG, &pctx)) {
+			fs->super->s_feature_incompat &=
+				~EXT2_FEATURE_INCOMPAT_META_BG;
+			fs->super->s_first_meta_bg = 0;
+			ext2fs_mark_super_dirty(fs);
+		}
+	}
+
 	/*
 	 * Verify the group descriptors....
 	 */
-- 
2.0.0

[PATCH 3/6] resize2fs: disable the meta_bg feature if necessary

[Theodore Ts'o]

When shrinking a file system, if the number block groups drops below
the point where we started using the meta_bg layout, disable the
meta_bg feature and set s_first_meta_bg to zero.  This is necessary to
avoid creating an invalid/corrupted file system after the shrink.

Addresses-Debian-Bug: 756922

Reported-by: Marcin Wolcendorf <antymat+debian@chelmska.waw.pl>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 resize/resize2fs.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index 9641b1e..b3755f6 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -489,6 +489,13 @@ retry:
 		fs->super->s_reserved_gdt_blocks = new;
 	}
 
+	if ((fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG) &&
+	    (fs->super->s_first_meta_bg > fs->desc_blocks)) {
+		fs->super->s_feature_incompat &=
+			~EXT2_FEATURE_INCOMPAT_META_BG;
+		fs->super->s_first_meta_bg = 0;
+	}
+
 	/*
 	 * Update the location of the backup superblocks if the
 	 * sparse_super2 feature is enabled.
@@ -998,13 +1005,15 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
 		ext2fs_mark_block_bitmap2(rfs->reserve_blocks, blk);
 	}
 
-	if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG) {
+	if (old_fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
 		old_blocks = old_fs->super->s_first_meta_bg;
+	else
+		old_blocks = old_fs->desc_blocks +
+			old_fs->super->s_reserved_gdt_blocks;
+	if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
 		new_blocks = fs->super->s_first_meta_bg;
-	} else {
-		old_blocks = old_fs->desc_blocks + old_fs->super->s_reserved_gdt_blocks;
+	else
 		new_blocks = fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
-	}
 
 	retval = reserve_sparse_super2_last_group(rfs, meta_bmap);
 	if (retval)
-- 
2.0.0

[PATCH 4/6] tests: make sure MKE2FS_FIRST_META_BG is unset while running tests

[Theodore Ts'o]

If the developer has set the MKE2FS_FIRST_META_BG environment
variable, this can cause test failures.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/test_config | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test_config b/tests/test_config
index 6789656..1872e5c 100644
--- a/tests/test_config
+++ b/tests/test_config
@@ -32,6 +32,7 @@ E2FSCK_CONFIG=/dev/null
 export E2FSCK_CONFIG
 MKE2FS_CONFIG=./mke2fs.conf
 export MKE2FS_CONFIG
+unset MKE2FS_FIRST_META_BG
 E2FSPROGS_SKIP_PROGRESS=yes
 export E2FSPROGS_SKIP_PROGRESS
 EXT2FS_NO_MTAB_OK=yes
-- 
2.0.0

[PATCH 5/6] tests: add f_first_meta_bg_too_big test

[Theodore Ts'o]

The test verifies that e2fsck can properly fix a file system where the
value of s_first_meta_bg in the superblock is larger than the number
of block group descriptors in the file system.  E2fsck will fix this
by clearing the meta_bg feature.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/f_first_meta_bg_too_big/expect.1 |  20 ++++++++++++++++++++
 tests/f_first_meta_bg_too_big/expect.2 |   7 +++++++
 tests/f_first_meta_bg_too_big/image.gz | Bin 0 -> 533 bytes
 tests/f_first_meta_bg_too_big/name     |   1 +
 4 files changed, 28 insertions(+)
 create mode 100644 tests/f_first_meta_bg_too_big/expect.1
 create mode 100644 tests/f_first_meta_bg_too_big/expect.2
 create mode 100644 tests/f_first_meta_bg_too_big/image.gz
 create mode 100644 tests/f_first_meta_bg_too_big/name

diff --git a/tests/f_first_meta_bg_too_big/expect.1 b/tests/f_first_meta_bg_too_big/expect.1
new file mode 100644
index 0000000..85786bd
--- /dev/null
+++ b/tests/f_first_meta_bg_too_big/expect.1
@@ -0,0 +1,20 @@
+First_meta_bg is too big.  (2, max value 1).  Clear? yes
+
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+Block bitmap differences:  -3
+Fix? yes
+
+Free blocks count wrong for group #0 (79, counted=80).
+Fix? yes
+
+Free blocks count wrong (79, counted=80).
+Fix? yes
+
+
+test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
+test_filesys: 11/16 files (0.0% non-contiguous), 20/100 blocks
+Exit status is 1
diff --git a/tests/f_first_meta_bg_too_big/expect.2 b/tests/f_first_meta_bg_too_big/expect.2
new file mode 100644
index 0000000..44bff62
--- /dev/null
+++ b/tests/f_first_meta_bg_too_big/expect.2
@@ -0,0 +1,7 @@
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+test_filesys: 11/16 files (0.0% non-contiguous), 20/100 blocks
+Exit status is 0
diff --git a/tests/f_first_meta_bg_too_big/image.gz b/tests/f_first_meta_bg_too_big/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..527937040de93a57428e73c8aee9181212b5efc4
GIT binary patch
literal 533
zc-oWi=HNKx`z)A=IWspgJ(c0@-Tm3Zfiesq%BLTb_idY2_ClrAs>5kT^Xe-*MGpFu
z3A8^_NNQiD!NJBV{_;V}+`q;xmyW9L_#wVSR6H=?qU*WozZt7~tu&-2ooC<ov*_gR
z??uyp@AGEb*}~wnYNmv=;Vi>*(=C7QJD4PB{4}XPcfYjLw9@;kmAjL62CTi_b+Nlp
z=~$~ti?yNst95qu-%_4_KHPnL+xfWI|G(_Fea+jp`|rz-r~X{N`Bj`hE;dTa=6+ev
zmTj{2!A4K^t&~WYsp@GsxY0uXaYfw!vwOs}&ThE6y*=U2T)pD_@1^s~^XAr6{=avy
z?DW<-f0o@3Tl;_2{i8*5Za*!WT7T{T?@fQ7#!Y;l)gS)%{nWRwrk#p8o4+|Y@x_l)
z$)2r?6BpWC+kZpSAn?*3<AO<Lr;3xUmt-yvp0oYF$+FAYv!l)ZVsGxLtUX+<B4NHZ
z>Ti9H$X9-!3)34Kez9M%U+|CdEB}gmhri6P<OBYLg%}tNiZ&gtW9FTFiid*%1uR~^
zf7|`FkK+4&e~US>-oC8hkK4ccBZ@L7^OxJJ*vBl>>Ad)56JvcszQKPT{@L}HHhk#*
axm@<~guCnvlz^Uv`G<RLRx?asWB>r>%kgsn

literal 0
Hc-jL100001

diff --git a/tests/f_first_meta_bg_too_big/name b/tests/f_first_meta_bg_too_big/name
new file mode 100644
index 0000000..47ec96d
--- /dev/null
+++ b/tests/f_first_meta_bg_too_big/name
@@ -0,0 +1 @@
+s_first_meta_bg is too large
-- 
2.0.0

[PATCH 6/6] tests: add the r_meta_bg_shrink test

[Theodore Ts'o]

This test checks to make sure resize2fs can properly handle a file
system which started life as a normal ext4 file system and then was
grown to a size where meta_bg was enabled, and then shrunk back below
the point where the meta_bg format is still needed.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/r_meta_bg_shrink/script | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 tests/r_meta_bg_shrink/script

diff --git a/tests/r_meta_bg_shrink/script b/tests/r_meta_bg_shrink/script
new file mode 100644
index 0000000..a29c28a
--- /dev/null
+++ b/tests/r_meta_bg_shrink/script
@@ -0,0 +1,34 @@
+if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
+
+test_description="meta_bg shrink"
+FEATURES="-t ext4 -O 64bit,meta_bg,^resize_inode -b 1024"
+SIZE_1=1G
+SIZE_2=48M
+LOG=$test_name.log
+E2FSCK=../e2fsck/e2fsck
+RESIZE2FS_OPTS=-f
+
+. $cmd_dir/scripts/resize_test
+
+export MKE2FS_FIRST_META_BG=2
+resize_test
+unset MKE2FS_FIRST_META_BG
+
+RC=$?
+if [ $RC -eq 0 ]; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+elif [ $RC -eq 111 ]; then
+	echo "$test_name: $test_description: skipped"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	ln $LOG $test_name.failed
+fi
+
+unset FEATURES SIZE_1 SIZE_2 LOG E2FSCK
+
+else #if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
+	echo "$test_name: $test_description: skipped"
+fi 
+
-- 
2.0.0