linux-ext4 - Re: [PATCH 07/51] e2fsck: Verify and correct inode checksums

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20111219201221.GL8233@tux1.beaverton.ibm.com>
Date:	Mon, 19 Dec 2011 12:12:21 -0800
From:	"Darrick J. Wong" <djwong@...ibm.com>
To:	Andreas Dilger <adilger.kernel@...ger.ca>
Cc:	Theodore Tso <tytso@....edu>,
	Sunil Mushran <sunil.mushran@...cle.com>,
	Amir Goldstein <amir73il@...il.com>,
	Andi Kleen <andi@...stfloor.org>,
	Mingming Cao <cmm@...ibm.com>,
	Joel Becker <jlbec@...lplan.org>, linux-ext4@...r.kernel.org,
	Coly Li <colyli@...il.com>
Subject: Re: [PATCH 07/51] e2fsck: Verify and correct inode checksums

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html