Patchwork [08/54] e2fsck: Verify and correct inode checksums

login
register
mail settings
Submitter Darrick J. Wong
Date March 6, 2012, 11:58 p.m.
Message ID <20120306235812.11945.3055.stgit@elm3b70.beaverton.ibm.com>
Download mbox | patch
Permalink /patch/145069/
State Deferred
Delegated to: Theodore Ts'o
Headers show

Comments

Darrick J. Wong - March 6, 2012, 11:58 p.m.
Detect mismatches of the inode and checksum, and prompt the user to fix the
situation.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
 e2fsck/pass1.c   |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 e2fsck/problem.c |   10 +++++++++
 e2fsck/problem.h |    6 +++++
 3 files changed, 75 insertions(+), 1 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o - March 10, 2012, 5:09 a.m.
On Tue, Mar 06, 2012 at 03:58:12PM -0800, Darrick J. Wong wrote:
> Detect mismatches of the inode and checksum, and prompt the user to fix the
> situation.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>

The new problem definitions were inserted into the wrong place in
problem.c.  This is something that was picked up by a unit test in the
e2fsck directory:

LD_LIBRARY_PATH=../lib DYLD_LIBRARY_PATH=../lib ./tst_problem
*** Unordered problem table:
curr code = 0x00010065: The bad @b @i looks @n.  
*** prev code = 0x00010067: @i %i passes checks, but checksum does not match @i.  
*** This is a programming error in e2fsck
make[1]: *** [check] Error 1

The fix was simple; just move the new lines in problem.c down two lines.

>  	/* Quota inode is user visible */
>  	{ PR_1_QUOTA_INODE_NOT_HIDDEN,
>  	  N_("@q @i is visible to the user.  "),
> - 	  PROMPT_CLEAR, PR_PREEN_OK },
> +	  PROMPT_FIX, PR_PREEN_OK },

PR1_QUOTA_INODE_NOT_HIDDEN isn't currently used, so there's no point
changing it now.  Depending on how this bit of the first class quota
patch gets implemented, the message might change completely.

      	   		    	    	  	 - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong - March 12, 2012, 10:49 p.m.
On Sat, Mar 10, 2012 at 12:09:28AM -0500, Ted Ts'o wrote:
> On Tue, Mar 06, 2012 at 03:58:12PM -0800, Darrick J. Wong wrote:
> > Detect mismatches of the inode and checksum, and prompt the user to fix the
> > situation.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> 
> The new problem definitions were inserted into the wrong place in
> problem.c.  This is something that was picked up by a unit test in the
> e2fsck directory:
> 
> LD_LIBRARY_PATH=../lib DYLD_LIBRARY_PATH=../lib ./tst_problem
> *** Unordered problem table:
> curr code = 0x00010065: The bad @b @i looks @n.  
> *** prev code = 0x00010067: @i %i passes checks, but checksum does not match @i.  
> *** This is a programming error in e2fsck
> make[1]: *** [check] Error 1
> 
> The fix was simple; just move the new lines in problem.c down two lines.
> 
> >  	/* Quota inode is user visible */
> >  	{ PR_1_QUOTA_INODE_NOT_HIDDEN,
> >  	  N_("@q @i is visible to the user.  "),
> > - 	  PROMPT_CLEAR, PR_PREEN_OK },
> > +	  PROMPT_FIX, PR_PREEN_OK },
> 
> PR1_QUOTA_INODE_NOT_HIDDEN isn't currently used, so there's no point
> changing it now.  Depending on how this bit of the first class quota
> patch gets implemented, the message might change completely.

Ugh, that looks like a merge error on my end.  Sorry and thanks for fixing it.

--D

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index c31e073..471e3e0 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -540,6 +540,40 @@  extern void e2fsck_setup_tdb_icount(e2fsck_t ctx, int flags,
 		*ret = 0;
 }
 
+static errcode_t recheck_bad_inode_checksum(ext2_filsys fs, ext2_ino_t ino,
+					    e2fsck_t ctx,
+					    struct problem_context *pctx)
+{
+	errcode_t retval;
+	struct ext2_inode_large inode;
+
+	/*
+	 * Reread inode.  If we don't see checksum error, then this inode
+	 * has been fixed elsewhere.
+	 */
+	retval = ext2fs_read_inode_full(fs, ino, (struct ext2_inode *)&inode,
+					sizeof(inode));
+	if (retval && retval != EXT2_ET_INODE_CSUM_INVALID)
+		return retval;
+	if (!retval)
+		return 0;
+
+	/*
+	 * Checksum still doesn't match.  That implies that the inode passes
+	 * all the sanity checks, so maybe the checksum is simply corrupt.
+	 * See if the user will go for fixing that.
+	 */
+	if (!fix_problem(ctx, PR_1_INODE_ONLY_CSUM_INVALID, pctx))
+		return 0;
+
+	retval = ext2fs_write_inode_full(fs, ino, (struct ext2_inode *)&inode,
+					 sizeof(inode));
+	if (retval)
+		return retval;
+
+	return 0;
+}
+
 void e2fsck_pass1(e2fsck_t ctx)
 {
 	int	i;
@@ -561,6 +595,7 @@  void e2fsck_pass1(e2fsck_t ctx)
 	int		imagic_fs, extent_fs;
 	int		busted_fs_time = 0;
 	int		inode_size;
+	int		failed_csum = 0;
 
 	init_resource_track(&rtrack, ctx->fs->io);
 	clear_problem_context(&pctx);
@@ -740,7 +775,8 @@  void e2fsck_pass1(e2fsck_t ctx)
 			ext2fs_mark_inode_bitmap2(ctx->inode_used_map, ino);
 			continue;
 		}
-		if (pctx.errcode) {
+		if (pctx.errcode &&
+		    pctx.errcode != EXT2_ET_INODE_CSUM_INVALID) {
 			fix_problem(ctx, PR_1_ISCAN_ERROR, &pctx);
 			ctx->flags |= E2F_FLAG_ABORT;
 			return;
@@ -750,6 +786,14 @@  void e2fsck_pass1(e2fsck_t ctx)
 		pctx.ino = ino;
 		pctx.inode = inode;
 		ctx->stashed_ino = ino;
+
+		/* Clear corrupt inode? */
+		if (pctx.errcode == EXT2_ET_INODE_CSUM_INVALID) {
+			if (fix_problem(ctx, PR_1_INODE_CSUM_INVALID, &pctx))
+				goto clear_inode;
+			failed_csum = 1;
+		}
+
 		if (inode->i_links_count) {
 			pctx.errcode = ext2fs_icount_store(ctx->inode_link_info,
 					   ino, inode->i_links_count);
@@ -1146,6 +1190,20 @@  void e2fsck_pass1(e2fsck_t ctx)
 		} else
 			check_blocks(ctx, &pctx, block_buf);
 
+		/*
+		 * If the inode failed the checksum and the user didn't
+		 * clear the inode, test the checksum again -- if it still
+		 * fails, ask the user if the checksum should be corrected.
+		 */
+		if (failed_csum) {
+			pctx.errcode = recheck_bad_inode_checksum(fs, ino, ctx,
+								  &pctx);
+			if (pctx.errcode) {
+				ctx->flags |= E2F_FLAG_ABORT;
+				return;
+			}
+		}
+
 		if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
 			return;
 
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 579c838..a6dd88a 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -929,8 +929,18 @@  static struct e2fsck_problem problem_table[] = {
 	/* Quota inode is user visible */
 	{ PR_1_QUOTA_INODE_NOT_HIDDEN,
 	  N_("@q @i is visible to the user.  "),
+	  PROMPT_FIX, PR_PREEN_OK },
+
+	/* inode checksum does not match inode */
+	{ PR_1_INODE_CSUM_INVALID,
+	  N_("@i %i checksum does not match @i.  "),
 	  PROMPT_CLEAR, PR_PREEN_OK },
 
+	/* inode passes checks, but checksum does not match inode */
+	{ PR_1_INODE_ONLY_CSUM_INVALID,
+	  N_("@i %i passes checks, but checksum does not match @i.  "),
+	  PROMPT_FIX, PR_PREEN_OK },
+
 	/* Invalid bad inode */
 	{ PR_1_INVALID_BAD_INODE,
 	  N_("The bad @b @i looks @n.  "),
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index 3fabc90..5797fe2 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -550,6 +550,12 @@  struct problem_context {
 /* Invalid bad inode */
 #define PR_1_INVALID_BAD_INODE		0x010065
 
+/* inode checksum does not match inode */
+#define PR_1_INODE_CSUM_INVALID		0x010066
+
+/* inode passes checks, but checksum does not match inode */
+#define PR_1_INODE_ONLY_CSUM_INVALID	0x010067
+
 /*
  * Pass 1b errors
  */