(unknown)

(unknown)

[Eric Sesterhenn]

As suggested by pavel i tested how e2fsck handles corrupted images,
I used the fuzzer bunny (http://code.google.com/p/bunny-the-fuzzer/)
At http://www.cccmz.de/~snakebyte/e2fsck_err.tar.bz2 you
can find a bunch of images crashing e2fsck or keeping it in an endless
loop. I tested with e2fsck 1.41.0 which was the one i had at hand.

The crashes are either in ext2fs_inode_alloc_stats2() or
ext2fs_read_inode_full(), looks like those are always the same
faults.

000	endless loop
000	endless loop
053	ext2fs_inode_alloc_stats2
054	ext2fs_inode_alloc_stats2
073	different endless loop?
086	ext2fs_inode_alloc_stats2
112	ext2fs_read_inode_full
139	ext2fs_inode_alloc_stats2
143	ext2fs_inode_alloc_stats2
161	ext2fs_inode_alloc_stats2
192	ext2fs_inode_alloc_stats2
209	ext2fs_inode_alloc_stats2
214	endless loop
216	ext2fs_read_inode_full
241	endless loop
266	endless loop
303	ext2fs_inode_alloc_stats2
389	ext2fs_inode_alloc_stats2
438	ext2fs_inode_alloc_stats2
440	endless loop
446	ext2fs_inode_alloc_stats2
449	ext2fs_read_inode_full
451	ext2fs_read_inode_full
455	ext2fs_read_inode_full
518	ext2fs_inode_alloc_stats2
530	ext2fs_inode_alloc_stats2
534	ext2fs_inode_alloc_stats2

Greetings, Eric

Re: e2fsck faults with corrupted images

[Eric Sesterhenn]

* Eric Sesterhenn (snakebyte@gmx.de) wrote:
> As suggested by pavel i tested how e2fsck handles corrupted images,
> I used the fuzzer bunny (http://code.google.com/p/bunny-the-fuzzer/)
> At http://www.cccmz.de/~snakebyte/e2fsck_err.tar.bz2 you
> can find a bunch of images crashing e2fsck or keeping it in an endless
> loop. I tested with e2fsck 1.41.0 which was the one i had at hand.
> 
> The crashes are either in ext2fs_inode_alloc_stats2() or
> ext2fs_read_inode_full(), looks like those are always the same
> faults.

I just tried again with e2sck 1.41.3 and I can still
produce the errors.

Greetings, Eric

Re: e2fsck faults with corrupted images

[Theodore Tso]

On Thu, Jan 22, 2009 at 12:12:01PM +0100, Eric Sesterhenn wrote:
> * Eric Sesterhenn (snakebyte@gmx.de) wrote:
> > As suggested by pavel i tested how e2fsck handles corrupted images,
> > I used the fuzzer bunny (http://code.google.com/p/bunny-the-fuzzer/)
> > At http://www.cccmz.de/~snakebyte/e2fsck_err.tar.bz2 you
> > can find a bunch of images crashing e2fsck or keeping it in an endless
> > loop. I tested with e2fsck 1.41.0 which was the one i had at hand.
> > 
> > The crashes are either in ext2fs_inode_alloc_stats2() or
> > ext2fs_read_inode_full(), looks like those are always the same
> > faults.

Thanks, they were all traced to the superblock parameter s_first_ino
being extremely large --- much larger than s_inodes_count.  I've
committed the following patches to address the problem at multiple
levels.

							- Ted

[PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid

[Theodore Ts'o]

An deliberately corrupted filesystem with an insanely large
s_first_ino field could cause e2fsck to crash with a seg fault.

Thanks to Eric Sesterhenn for supplying test cases which demonstrated
this issue.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 e2fsck/super.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/e2fsck/super.c b/e2fsck/super.c
index cd2b9f0..24ec7a8 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -513,6 +513,10 @@ void check_super_block(e2fsck_t ctx)
 	check_super_value(ctx, "reserved_gdt_blocks",
 			  sb->s_reserved_gdt_blocks, MAX_CHECK, 0,
 			  fs->blocksize/4);
+	if (sb->s_rev_level > EXT2_GOOD_OLD_REV)
+		check_super_value(ctx, "first_ino", sb->s_first_ino,
+				  MIN_CHECK | MAX_CHECK,
+				  EXT2_GOOD_OLD_FIRST_INO, sb->s_inodes_count);
 	inode_size = EXT2_INODE_SIZE(sb);
 	check_super_value(ctx, "inode_size",
 			  inode_size, MIN_CHECK | MAX_CHECK,
-- 
1.6.0.4.8.g36f27.dirty

[PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number

[Theodore Ts'o]

Add a sanity check to makesure that even if the superblock field
s_first_inode is insane, that we won't return an invalid inode number.
(The function will return the error EXT2_ET_INODE_ALLOC_FAIL in that
case.)

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/alloc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
index be2b56b..ade5149 100644
--- a/lib/ext2fs/alloc.c
+++ b/lib/ext2fs/alloc.c
@@ -120,6 +120,8 @@ errcode_t ext2fs_new_inode(ext2_filsys fs, ext2_ino_t dir,
 	start_inode = (dir_group * EXT2_INODES_PER_GROUP(fs->super)) + 1;
 	if (start_inode < EXT2_FIRST_INODE(fs->super))
 		start_inode = EXT2_FIRST_INODE(fs->super);
+	if (start_inode > fs->super->s_inodes_count)
+		return EXT2_ET_INODE_ALLOC_FAIL;
 	i = start_inode;
 
 	do {
-- 
1.6.0.4.8.g36f27.dirty

[PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats

[Theodore Ts'o]

If ext2fs_inode_alloc_stats2() or ext2fs_block_alloc_stats() is passed
an insanely large inode or block number, it's possible for these
functions to overrun an array boundary and cause the calling program
to crash with a memory error.

Detect this case, and since these functions don't return an error
code, print a warning message, much like we do in ext2fs_warn_bitmap2().

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/alloc_stats.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/lib/ext2fs/alloc_stats.c b/lib/ext2fs/alloc_stats.c
index 97661dc..d523b43 100644
--- a/lib/ext2fs/alloc_stats.c
+++ b/lib/ext2fs/alloc_stats.c
@@ -20,6 +20,13 @@ void ext2fs_inode_alloc_stats2(ext2_filsys fs, ext2_ino_t ino,
 {
 	int	group = ext2fs_group_of_ino(fs, ino);
 
+#ifndef OMIT_COM_ERR
+	if (ino > fs->super->s_inodes_count) {
+		com_err("ext2fs_inode_alloc_stats2", 0,
+			"Illegal inode number: %lu", ino);
+		return;
+	}
+#endif
 	if (inuse > 0)
 		ext2fs_mark_inode_bitmap(fs->inode_map, ino);
 	else
@@ -58,6 +65,13 @@ void ext2fs_block_alloc_stats(ext2_filsys fs, blk_t blk, int inuse)
 {
 	int	group = ext2fs_group_of_blk(fs, blk);
 
+#ifndef OMIT_COM_ERR
+	if (blk >= fs->super->s_blocks_count) {
+		com_err("ext2fs_block_alloc_stats2", 0,
+			"Illegal block number: %lu", blk);
+		return;
+	}
+#endif
 	if (inuse > 0)
 		ext2fs_mark_block_bitmap(fs->block_map, blk);
 	else
-- 
1.6.0.4.8.g36f27.dirty

[PATCH] e2fsck: Change PR_3_CREATE_LPF_ERROR to be a non-fatal problem

[Theodore Ts'o]

The other problem codes associated with failing to create the
lost+found directory are non-fatal, and this one should be non-fatal
as well.  The two places which call e2fsck_get_lost_and_found()
already deal with a failure to create the directory, so there's no
point making this be a fatal error.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 e2fsck/problem.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 9d193c3..3f53350 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -1386,7 +1386,7 @@ static struct e2fsck_problem problem_table[] = {
 	/* Error creating lost and found directory */
 	{ PR_3_CREATE_LPF_ERROR,
 	  N_("Error creating /@l @d (%s): %m\n"),
-	  PROMPT_NONE, PR_FATAL },
+	  PROMPT_NONE, 0 },
 
 	/* Root inode is not directory; aborting */
 	{ PR_3_ROOT_NOT_DIR_ABORT,
-- 
1.6.0.4.8.g36f27.dirty

[PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid

[Theodore Ts'o]

An deliberately corrupted filesystem with an insanely large
s_first_ino field could cause e2fsck to crash with a seg fault.

Thanks to Eric Sesterhenn for supplying test cases which demonstrated
this issue.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 e2fsck/super.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/e2fsck/super.c b/e2fsck/super.c
index cd2b9f0..24ec7a8 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -513,6 +513,10 @@ void check_super_block(e2fsck_t ctx)
 	check_super_value(ctx, "reserved_gdt_blocks",
 			  sb->s_reserved_gdt_blocks, MAX_CHECK, 0,
 			  fs->blocksize/4);
+	if (sb->s_rev_level > EXT2_GOOD_OLD_REV)
+		check_super_value(ctx, "first_ino", sb->s_first_ino,
+				  MIN_CHECK | MAX_CHECK,
+				  EXT2_GOOD_OLD_FIRST_INO, sb->s_inodes_count);
 	inode_size = EXT2_INODE_SIZE(sb);
 	check_super_value(ctx, "inode_size",
 			  inode_size, MIN_CHECK | MAX_CHECK,
-- 
1.6.0.4.8.g36f27.dirty

[PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number

[Theodore Ts'o]

Add a sanity check to makesure that even if the superblock field
s_first_inode is insane, that we won't return an invalid inode number.
(The function will return the error EXT2_ET_INODE_ALLOC_FAIL in that
case.)

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/alloc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
index be2b56b..ade5149 100644
--- a/lib/ext2fs/alloc.c
+++ b/lib/ext2fs/alloc.c
@@ -120,6 +120,8 @@ errcode_t ext2fs_new_inode(ext2_filsys fs, ext2_ino_t dir,
 	start_inode = (dir_group * EXT2_INODES_PER_GROUP(fs->super)) + 1;
 	if (start_inode < EXT2_FIRST_INODE(fs->super))
 		start_inode = EXT2_FIRST_INODE(fs->super);
+	if (start_inode > fs->super->s_inodes_count)
+		return EXT2_ET_INODE_ALLOC_FAIL;
 	i = start_inode;
 
 	do {
-- 
1.6.0.4.8.g36f27.dirty

[PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats

[Theodore Ts'o]

If ext2fs_inode_alloc_stats2() or ext2fs_block_alloc_stats() is passed
an insanely large inode or block number, it's possible for these
functions to overrun an array boundary and cause the calling program
to crash with a memory error.

Detect this case, and since these functions don't return an error
code, print a warning message, much like we do in ext2fs_warn_bitmap2().

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/alloc_stats.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/lib/ext2fs/alloc_stats.c b/lib/ext2fs/alloc_stats.c
index 97661dc..d523b43 100644
--- a/lib/ext2fs/alloc_stats.c
+++ b/lib/ext2fs/alloc_stats.c
@@ -20,6 +20,13 @@ void ext2fs_inode_alloc_stats2(ext2_filsys fs, ext2_ino_t ino,
 {
 	int	group = ext2fs_group_of_ino(fs, ino);
 
+#ifndef OMIT_COM_ERR
+	if (ino > fs->super->s_inodes_count) {
+		com_err("ext2fs_inode_alloc_stats2", 0,
+			"Illegal inode number: %lu", ino);
+		return;
+	}
+#endif
 	if (inuse > 0)
 		ext2fs_mark_inode_bitmap(fs->inode_map, ino);
 	else
@@ -58,6 +65,13 @@ void ext2fs_block_alloc_stats(ext2_filsys fs, blk_t blk, int inuse)
 {
 	int	group = ext2fs_group_of_blk(fs, blk);
 
+#ifndef OMIT_COM_ERR
+	if (blk >= fs->super->s_blocks_count) {
+		com_err("ext2fs_block_alloc_stats2", 0,
+			"Illegal block number: %lu", blk);
+		return;
+	}
+#endif
 	if (inuse > 0)
 		ext2fs_mark_block_bitmap(fs->block_map, blk);
 	else
-- 
1.6.0.4.8.g36f27.dirty

[PATCH] e2fsck: Change PR_3_CREATE_LPF_ERROR to be a non-fatal problem

[Theodore Ts'o]

The other problem codes associated with failing to create the
lost+found directory are non-fatal, and this one should be non-fatal
as well.  The two places which call e2fsck_get_lost_and_found()
already deal with a failure to create the directory, so there's no
point making this be a fatal error.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 e2fsck/problem.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 9d193c3..3f53350 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -1386,7 +1386,7 @@ static struct e2fsck_problem problem_table[] = {
 	/* Error creating lost and found directory */
 	{ PR_3_CREATE_LPF_ERROR,
 	  N_("Error creating /@l @d (%s): %m\n"),
-	  PROMPT_NONE, PR_FATAL },
+	  PROMPT_NONE, 0 },
 
 	/* Root inode is not directory; aborting */
 	{ PR_3_ROOT_NOT_DIR_ABORT,
-- 
1.6.0.4.8.g36f27.dirty