LinuxLists.cc - (unknown)

2009-01-22 08:56:13

Subject: (unknown)

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

2009-01-22 11:13:15

by Eric Sesterhenn

[permalink] [raw]

Subject: Re: e2fsck faults with corrupted images

* Eric Sesterhenn ([email protected]) 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

2009-01-22 21:12:30

by Theodore Ts'o

[permalink] [raw]

Subject: Re: e2fsck faults with corrupted images

On Thu, Jan 22, 2009 at 12:12:01PM +0100, Eric Sesterhenn wrote:
> * Eric Sesterhenn ([email protected]) 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

2009-01-22 21:32:30

by Theodore Ts'o

[permalink] [raw]

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

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" <[email protected]>
---
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

2009-01-22 21:32:30

by Theodore Ts'o

[permalink] [raw]

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

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" <[email protected]>
---
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

2009-01-22 21:32:30

by Theodore Ts'o

[permalink] [raw]

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

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" <[email protected]>
---
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

2009-01-22 21:32:30

by Theodore Ts'o

[permalink] [raw]

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

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" <[email protected]>
---
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

2009-01-22 21:33:07

by Theodore Ts'o

[permalink] [raw]

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

2009-01-22 21:33:07

by Theodore Ts'o

[permalink] [raw]

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

2009-01-22 21:33:07

by Theodore Ts'o

[permalink] [raw]

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

2009-01-22 21:33:07

by Theodore Ts'o

[permalink] [raw]