Turns out a "should never happen" error can indeed happen very easily
if a directory with an htree index has an incorrect, and too-large,
i_size field. This patch fixes this so that we handle this situation
gracefully, allowing filesystems with this error to be fixed.
In another patch I will clean up the specific problem which caused the
internal "should never happen" error from happening at all, but patch
will prevent e2fsck from crashing, and prompt the user to remove the
htree index, so it can be rebuilt again after pass 3.
Thanks to Bas van Schaik at Tetra for giving me access to his system
so this problem could be debugged.
Addresses-Launchpad-Bug: #129395
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
e2fsck/pass2.c | 11 +++++++++--
e2fsck/problem.c | 4 ++++
e2fsck/problem.h | 3 +++
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
index 6fb2ed7..b79461d 100644
--- a/e2fsck/pass2.c
+++ b/e2fsck/pass2.c
@@ -783,8 +783,14 @@ static int check_dir_block(ext2_filsys fs,
dx_dir = e2fsck_get_dx_dir_info(ctx, ino);
if (dx_dir && dx_dir->numblocks) {
if (db->blockcnt >= dx_dir->numblocks) {
- printf("XXX should never happen!!!\n");
- abort();
+ if (fix_problem(ctx, PR_2_UNEXPECTED_HTREE_BLOCK,
+ &pctx)) {
+ clear_htree(ctx, ino);
+ dx_dir->numblocks = 0;
+ dx_db = 0;
+ goto out_htree;
+ }
+ fatal_error(ctx, _("Can not continue."));
}
dx_db = &dx_dir->dx_block[db->blockcnt];
dx_db->type = DX_DIRBLOCK_LEAF;
@@ -819,6 +825,7 @@ static int check_dir_block(ext2_filsys fs,
sizeof(struct ext2_dx_entry))))
dx_db->type = DX_DIRBLOCK_NODE;
}
+out_htree:
#endif /* ENABLE_HTREE */
dict_init(&de_dict, DICTCOUNT_T_MAX, dict_de_cmp);
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 7c3ebea..418d83b 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -1188,6 +1188,10 @@ static struct e2fsck_problem problem_table[] = {
N_("i_blocks_hi @F %N, @s zero.\n"),
PROMPT_CLEAR, 0 },
+ /* Unexpected HTREE block */
+ { PR_2_UNEXPECTED_HTREE_BLOCK,
+ N_("Unexpected @b in @h %d (%q).\n"), PROMPT_CLEAR_HTREE, 0 },
+
/* Pass 3 errors */
/* Pass 3: Checking directory connectivity */
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index f5f7212..91a6148 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -708,6 +708,9 @@ struct problem_context {
/* i_blocks_hi should be zero */
#define PR_2_BLOCKS_HI_ZERO 0x020044
+/* Unexpected HTREE block */
+#define PR_2_UNEXPECTED_HTREE_BLOCK 0x020045
+
/*
* Pass 3 errors
*/
--
1.5.4.1.144.gdfee-dirty
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
misc/e2image.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/misc/e2image.c b/misc/e2image.c
index 4313cc0..e10554e 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -579,8 +579,11 @@ static void install_image(char *device, char *image_fn, int raw_flag)
exit(1);
}
Add two new functions which allows the caller to examine the last
directory block entry added to the list, and to drop if it necessary.
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
lib/ext2fs/dblist.c | 26 ++++++++++++++++++++++++++
lib/ext2fs/ext2_err.et.in | 3 +++
lib/ext2fs/ext2fs.h | 3 +++
3 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/lib/ext2fs/dblist.c b/lib/ext2fs/dblist.c
index 21b36aa..3bf63a0 100644
--- a/lib/ext2fs/dblist.c
+++ b/lib/ext2fs/dblist.c
@@ -259,3 +259,29 @@ int ext2fs_dblist_count(ext2_dblist dblist)
{
return (int) dblist->count;
}
+
+errcode_t ext2fs_dblist_get_last(ext2_dblist dblist,
+ struct ext2_db_entry **entry)
+{
+ errcode_t retval;
+
+ EXT2_CHECK_MAGIC(dblist, EXT2_ET_MAGIC_DBLIST);
+
+ if (dblist->count == 0)
+ return EXT2_ET_DBLIST_EMPTY;
+
+ if (entry)
+ *entry = dblist->list + ( (int) dblist->count-1);
+ return 0;
+}
+
+errcode_t ext2fs_dblist_drop_last(ext2_dblist dblist)
+{
+ EXT2_CHECK_MAGIC(dblist, EXT2_ET_MAGIC_DBLIST);
+
+ if (dblist->count == 0)
+ return EXT2_ET_DBLIST_EMPTY;
+
+ dblist->count--;
+ return 0;
+}
diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in
index eda4bb4..7451242 100644
--- a/lib/ext2fs/ext2_err.et.in
+++ b/lib/ext2fs/ext2_err.et.in
@@ -326,5 +326,8 @@ ec EXT2_ET_TDB_ERR_NOEXIST,
ec EXT2_ET_TDB_ERR_RDONLY,
"TDB: Write not permitted"
+ec EXT2_ET_DBLIST_EMPTY,
+ "Ext2fs directory block list is empty"
+
end
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 0b2c321..d098961 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -652,6 +652,9 @@ extern errcode_t ext2fs_set_dir_block(ext2_dblist dblist, ext2_ino_t ino,
extern errcode_t ext2fs_copy_dblist(ext2_dblist src,
ext2_dblist *dest);
extern int ext2fs_dblist_count(ext2_dblist dblist);
+extern errcode_t ext2fs_dblist_get_last(ext2_dblist dblist,
+ struct ext2_db_entry **entry);
+extern errcode_t ext2fs_dblist_drop_last(ext2_dblist dblist);
/* dblist_dir.c */
extern errcode_t
--
1.5.4.1.144.gdfee-dirty
If a directory's i_size is bigger than the number of blocks, don't try
to allocate extra empty blocks to the end of the directory; there's no
real point to do that. Also, if a directory's i_size is not a
multiple of the blocksize, flag that as a mistake so it can be fixed.
This more elegantly addresses the problem which was found on Bas van
Schaik's filesystem.
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
e2fsck/pass1.c | 25 ++++++++++++++++++++++++-
tests/f_holedir/expect.1 | 13 ++++++++++---
tests/f_holedir/expect.2 | 2 +-
3 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index b4db8ef..839f099 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -1546,6 +1546,26 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
goto out;
}
+ if (pb.is_dir) {
+ while (1) {
+ struct ext2_db_entry *entry;
+
+ if (ext2fs_dblist_get_last(fs->dblist, &entry) ||
+ (entry->ino != ino) ||
+ (entry->blk != 0) ||
+ (entry->blockcnt == 0))
+ break;
+ /* printf("Dropping ino %lu blk %lu blockcnt %d\n",
+ entry->ino, entry->blk, entry->blockcnt); */
+ ext2fs_dblist_drop_last(fs->dblist);
+ if (ext2fs_dblist_get_last(fs->dblist, &entry) ||
+ (entry->ino != ino))
+ pb.last_block--;
+ else
+ pb.last_block = entry->blockcnt;
+ }
+ }
+
if (inode->i_flags & EXT2_INDEX_FL) {
if (handle_htree(ctx, pctx, ino, inode, block_buf)) {
inode->i_flags &= ~EXT2_INDEX_FL;
@@ -1583,7 +1603,9 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
#endif
if (pb.is_dir) {
int nblock = inode->i_size >> EXT2_BLOCK_SIZE_BITS(fs->super);
- if (nblock > (pb.last_block + 1))
+ if (inode->i_size & (fs->blocksize - 1))
+ bad_size = 5;
+ else if (nblock > (pb.last_block + 1))
bad_size = 1;
else if (nblock < (pb.last_block + 1)) {
if (((pb.last_block + 1) - nblock) >
@@ -1745,6 +1767,7 @@ static int process_block(ext2_filsys fs,
printf("Missing block (#%d) in directory inode %lu!\n",
blockcnt, p->ino);
#endif
+ p->last_block = blockcnt;
goto mark_dir;
}
return 0;
diff --git a/tests/f_holedir/expect.1 b/tests/f_holedir/expect.1
index 05e0cbb..ad74fa6 100644
--- a/tests/f_holedir/expect.1
+++ b/tests/f_holedir/expect.1
@@ -15,12 +15,19 @@ Directory inode 11 has an unallocated block #3. Allocate? yes
Directory inode 11 has an unallocated block #6. Allocate? yes
-Directory inode 11 has an unallocated block #11. Allocate? yes
-
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
+Block bitmap differences: -21
+Fix? yes
+
+Free blocks count wrong for group #0 (78, counted=79).
+Fix? yes
+
+Free blocks count wrong (78, counted=79).
+Fix? yes
+
test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
-test_filesys: 11/32 files (9.1% non-contiguous), 22/100 blocks
+test_filesys: 11/32 files (9.1% non-contiguous), 21/100 blocks
Exit status is 1
diff --git a/tests/f_holedir/expect.2 b/tests/f_holedir/expect.2
index a821f87..4c0b4f2 100644
--- a/tests/f_holedir/expect.2
+++ b/tests/f_holedir/expect.2
@@ -3,5 +3,5 @@ Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
-test_filesys: 11/32 files (0.0% non-contiguous), 22/100 blocks
+test_filesys: 11/32 files (0.0% non-contiguous), 21/100 blocks
Exit status is 0
--
1.5.4.1.144.gdfee-dirty
On Mar 13, 2008 01:12 -0400, Theodore Ts'o wrote:
> --- a/e2fsck/problem.h
> +++ b/e2fsck/problem.h
> @@ -708,6 +708,9 @@ struct problem_context {
> /* i_blocks_hi should be zero */
> #define PR_2_BLOCKS_HI_ZERO 0x020044
>
> +/* Unexpected HTREE block */
> +#define PR_2_UNEXPECTED_HTREE_BLOCK 0x020045
Just an FYI - this problem number conflicts with the "uninit groups"
patch. Not a fatal problem, but I suspect that with patch-fuzz this
might apply without being noticed. There is also a problem number
added here in the ibadness-counter patch that should be incremented.
Alternately, if you make this one 0x020048 it won't conflict with either.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.