e2fsck: don't check inode flag values for deleted inodes

Commit 47b8941774 "e2fsck: make sure system.data xattr is present"
exposed a bug in e2fsck's pass 1 handling which caused the xfstests's
generic/079 to fail if the inline_data feature was enabled.  The
problem was that e2fsck was checking if an inode with inline data had
the system.xattr EA before checking to see if that inode was still in
use --- and this invariant isn't necessarily true for deleted inodes.
There were a number of other checks that were done too early that
could also potentially cause false positive complaints, although those
would normally only happen if a now-unused portion of the inode table
had gotten corrupted, or if tune2fs had disabled a particular file
system feature and there old, deleted inodes that had values
inconsistent with the new file system configuration.

Reported-by: Eric Whitney <enwlinux@gmail.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
This commit is contained in:
Theodore Ts'o 2017-02-17 10:42:17 -05:00
parent bf9f3b6d5b
commit 3fac78e658

View File

@ -1316,6 +1316,34 @@ void e2fsck_pass1(e2fsck_t ctx)
}
failed_csum = pctx.errcode != 0;
/*
* Check for inodes who might have been part of the
* orphaned list linked list. They should have gotten
* dealt with by now, unless the list had somehow been
* corrupted.
*
* FIXME: In the future, inodes which are still in use
* (and which are therefore) pending truncation should
* be handled specially. Right now we just clear the
* dtime field, and the normal e2fsck handling of
* inodes where i_size and the inode blocks are
* inconsistent is to fix i_size, instead of releasing
* the extra blocks. This won't catch the inodes that
* was at the end of the orphan list, but it's better
* than nothing. The right answer is that there
* shouldn't be any bugs in the orphan list handling. :-)
*/
if (inode->i_dtime && low_dtime_check &&
inode->i_dtime < ctx->fs->super->s_inodes_count) {
if (fix_problem(ctx, PR_1_LOW_DTIME, &pctx)) {
inode->i_dtime = inode->i_links_count ?
0 : ctx->now;
e2fsck_write_inode(ctx, ino, inode,
"pass1");
failed_csum = 0;
}
}
if (inode->i_links_count) {
pctx.errcode = ext2fs_icount_store(ctx->inode_link_info,
ino, inode->i_links_count);
@ -1325,6 +1353,19 @@ void e2fsck_pass1(e2fsck_t ctx)
ctx->flags |= E2F_FLAG_ABORT;
goto endit;
}
} else if ((ino >= EXT2_FIRST_INODE(fs->super)) &&
!quota_inum_is_reserved(fs, ino)) {
if (!inode->i_dtime && inode->i_mode) {
if (fix_problem(ctx,
PR_1_ZERO_DTIME, &pctx)) {
inode->i_dtime = ctx->now;
e2fsck_write_inode(ctx, ino, inode,
"pass1");
failed_csum = 0;
}
}
FINISH_INODE_LOOP(ctx, ino, &pctx, failed_csum);
continue;
}
/* Conflicting inlinedata/extents inode flags? */
@ -1641,48 +1682,7 @@ void e2fsck_pass1(e2fsck_t ctx)
continue;
}
/*
* Check for inodes who might have been part of the
* orphaned list linked list. They should have gotten
* dealt with by now, unless the list had somehow been
* corrupted.
*
* FIXME: In the future, inodes which are still in use
* (and which are therefore) pending truncation should
* be handled specially. Right now we just clear the
* dtime field, and the normal e2fsck handling of
* inodes where i_size and the inode blocks are
* inconsistent is to fix i_size, instead of releasing
* the extra blocks. This won't catch the inodes that
* was at the end of the orphan list, but it's better
* than nothing. The right answer is that there
* shouldn't be any bugs in the orphan list handling. :-)
*/
if (inode->i_dtime && low_dtime_check &&
inode->i_dtime < ctx->fs->super->s_inodes_count) {
if (fix_problem(ctx, PR_1_LOW_DTIME, &pctx)) {
inode->i_dtime = inode->i_links_count ?
0 : ctx->now;
e2fsck_write_inode(ctx, ino, inode,
"pass1");
failed_csum = 0;
}
}
/*
* This code assumes that deleted inodes have
* i_links_count set to 0.
*/
if (!inode->i_links_count) {
if (!inode->i_dtime && inode->i_mode) {
if (fix_problem(ctx,
PR_1_ZERO_DTIME, &pctx)) {
inode->i_dtime = ctx->now;
e2fsck_write_inode(ctx, ino, inode,
"pass1");
failed_csum = 0;
}
}
FINISH_INODE_LOOP(ctx, ino, &pctx, failed_csum);
continue;
}