* (unknown) @ 2009-01-22 8:56 Eric Sesterhenn 2009-01-22 11:12 ` e2fsck faults with corrupted images Eric Sesterhenn 0 siblings, 1 reply; 11+ messages in thread From: Eric Sesterhenn @ 2009-01-22 8:56 UTC (permalink / raw) To: pavel, tytso; +Cc: linux-ext4 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: e2fsck faults with corrupted images 2009-01-22 8:56 (unknown) Eric Sesterhenn @ 2009-01-22 11:12 ` Eric Sesterhenn 2009-01-22 21:12 ` Theodore Tso 0 siblings, 1 reply; 11+ messages in thread From: Eric Sesterhenn @ 2009-01-22 11:12 UTC (permalink / raw) To: pavel, tytso; +Cc: linux-ext4 * 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: e2fsck faults with corrupted images 2009-01-22 11:12 ` e2fsck faults with corrupted images Eric Sesterhenn @ 2009-01-22 21:12 ` Theodore Tso 2009-01-22 21:32 ` [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid Theodore Ts'o 2009-01-22 21:33 ` [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid Theodore Ts'o 0 siblings, 2 replies; 11+ messages in thread From: Theodore Tso @ 2009-01-22 21:12 UTC (permalink / raw) To: Eric Sesterhenn; +Cc: pavel, linux-ext4 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid 2009-01-22 21:12 ` Theodore Tso @ 2009-01-22 21:32 ` Theodore Ts'o 2009-01-22 21:32 ` [PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number Theodore Ts'o 2009-01-22 21:33 ` [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid Theodore Ts'o 1 sibling, 1 reply; 11+ messages in thread From: Theodore Ts'o @ 2009-01-22 21:32 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Eric Sesterhenn, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number 2009-01-22 21:32 ` [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid Theodore Ts'o @ 2009-01-22 21:32 ` Theodore Ts'o 2009-01-22 21:32 ` [PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats Theodore Ts'o 0 siblings, 1 reply; 11+ messages in thread From: Theodore Ts'o @ 2009-01-22 21:32 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Eric Sesterhenn, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats 2009-01-22 21:32 ` [PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number Theodore Ts'o @ 2009-01-22 21:32 ` Theodore Ts'o 2009-01-22 21:32 ` [PATCH] e2fsck: Change PR_3_CREATE_LPF_ERROR to be a non-fatal problem Theodore Ts'o 0 siblings, 1 reply; 11+ messages in thread From: Theodore Ts'o @ 2009-01-22 21:32 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Eric Sesterhenn, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] e2fsck: Change PR_3_CREATE_LPF_ERROR to be a non-fatal problem 2009-01-22 21:32 ` [PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats Theodore Ts'o @ 2009-01-22 21:32 ` Theodore Ts'o 0 siblings, 0 replies; 11+ messages in thread From: Theodore Ts'o @ 2009-01-22 21:32 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Eric Sesterhenn, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid 2009-01-22 21:12 ` Theodore Tso 2009-01-22 21:32 ` [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid Theodore Ts'o @ 2009-01-22 21:33 ` Theodore Ts'o 2009-01-22 21:33 ` [PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number Theodore Ts'o 1 sibling, 1 reply; 11+ messages in thread From: Theodore Ts'o @ 2009-01-22 21:33 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Eric Sesterhenn, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number 2009-01-22 21:33 ` [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid Theodore Ts'o @ 2009-01-22 21:33 ` Theodore Ts'o 2009-01-22 21:33 ` [PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats Theodore Ts'o 0 siblings, 1 reply; 11+ messages in thread From: Theodore Ts'o @ 2009-01-22 21:33 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Eric Sesterhenn, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats 2009-01-22 21:33 ` [PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number Theodore Ts'o @ 2009-01-22 21:33 ` Theodore Ts'o 2009-01-22 21:33 ` [PATCH] e2fsck: Change PR_3_CREATE_LPF_ERROR to be a non-fatal problem Theodore Ts'o 0 siblings, 1 reply; 11+ messages in thread From: Theodore Ts'o @ 2009-01-22 21:33 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Eric Sesterhenn, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] e2fsck: Change PR_3_CREATE_LPF_ERROR to be a non-fatal problem 2009-01-22 21:33 ` [PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats Theodore Ts'o @ 2009-01-22 21:33 ` Theodore Ts'o 0 siblings, 0 replies; 11+ messages in thread From: Theodore Ts'o @ 2009-01-22 21:33 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Eric Sesterhenn, 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2009-01-22 21:33 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-22 8:56 (unknown) Eric Sesterhenn
2009-01-22 11:12 ` e2fsck faults with corrupted images Eric Sesterhenn
2009-01-22 21:12 ` Theodore Tso
2009-01-22 21:32 ` [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid Theodore Ts'o
2009-01-22 21:32 ` [PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number Theodore Ts'o
2009-01-22 21:32 ` [PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats Theodore Ts'o
2009-01-22 21:32 ` [PATCH] e2fsck: Change PR_3_CREATE_LPF_ERROR to be a non-fatal problem Theodore Ts'o
2009-01-22 21:33 ` [PATCH] e2fsck: Add superblock check to make sure s_first_ino is valid Theodore Ts'o
2009-01-22 21:33 ` [PATCH] ext2fs_new_inode(): Add sanity check to assure a valid inode number Theodore Ts'o
2009-01-22 21:33 ` [PATCH] libext2fs: Add sanity checks to ext2fs_{block,inode}_alloc_stats Theodore Ts'o
2009-01-22 21:33 ` [PATCH] e2fsck: Change PR_3_CREATE_LPF_ERROR to be a non-fatal problem Theodore Ts'o
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.