* (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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).