Patchwork [07/51] e2fsck: Verify and correct inode checksums

login
register
mail settings
Submitter Darrick J. Wong
Date Dec. 14, 2011, 1:14 a.m.
Message ID <20111214011403.20947.54324.stgit@elm3c44.beaverton.ibm.com>
Download mbox | patch
Permalink /patch/131236/
State Superseded
Headers show

Comments

Darrick J. Wong - Dec. 14, 2011, 1:14 a.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   |    9 ++++++++-
 e2fsck/problem.c |    7 ++++++-
 e2fsck/problem.h |    3 +++
 3 files changed, 17 insertions(+), 2 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
Darrick J. Wong - Dec. 19, 2011, 8:12 p.m.
On Mon, Dec 19, 2011 at 11:28:24AM +0100, Andreas Dilger wrote:
> On 2011-12-14, at 2:14 AM, Darrick J. Wong wrote:
> > Detect mismatches of the inode and checksum, and prompt the user to fix the
> > situation.
> > 
> > @@ -739,6 +740,12 @@ 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 &&
> > +		    fix_problem(ctx, PR_1_INODE_CSUM_INVALID, &pctx))
> > +			goto clear_inode;
> 
> If the user enters "n" here to clear the inode, does it make sense to
> also ask if they want the checksum be corrected?  If this is also "n"
> (as is the case with "e2fsck -n") then nothing is done, but in some
> cases it makes sense to allow the user to keep the inode rather than
> only having the option to erase it.

I was thinking something like this might work:

if (csum_invalid)
	ask and then jump to clear_inode;

...all other inode sanity checks go here...

if (csum_was_invalid)
	ask and correct inode checksum;

That way, if the inode manages to survive all the other sanity checks, the user
has the opportunity to save the inode.  If the inode contains junk, then the
user will probably clear the inode on account of the garbage.

Come to think of it, this is probably a good idea for everything else.  I
think?

> > diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> > index f042b89..89dc72b 100644
> > --- a/e2fsck/problem.c
> > +++ b/e2fsck/problem.c
> > @@ -934,7 +934,12 @@ static struct e2fsck_problem problem_table[] = {
> > +	/* inode checksum does not match inode */
> > +	{ PR_1_INODE_CSUM_INVALID,
> > +	  N_("@i %i checksum does not match @i.  "),
> > +	  PROMPT_FIX, PR_PREEN_OK },
> 
> Also, "PROMPT_FIX" is misleading if the "fix" is to erase the inode.
> It should instead be "PROMPT_CLEAR_INODE".

Ok.

--D
> 
> Cheers, Andreas
> 
> 
> 
> 
> 

--
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 62e49c6..15320c6 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -729,7 +729,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;
@@ -739,6 +740,12 @@  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 &&
+		    fix_problem(ctx, PR_1_INODE_CSUM_INVALID, &pctx))
+			goto clear_inode;
+
 		if (inode->i_links_count) {
 			pctx.errcode = ext2fs_icount_store(ctx->inode_link_info,
 					   ino, inode->i_links_count);
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index f042b89..89dc72b 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -934,7 +934,12 @@  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_CLEAR, PR_PREEN_OK },
+	  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_FIX, PR_PREEN_OK },
 
 	/* Invalid bad inode */
 	{ PR_1_INVALID_BAD_INODE,
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index 9db29d8..f9f8cd7 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -550,6 +550,9 @@  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
+
 /*
  * Pass 1b errors
  */