Date:	Fri, 29 Nov 2013 15:11:44 -0700
From:	Andreas Dilger <adilger@...ger.ca>
To:	David Turner <novalis@...alis.org>
Cc:	Mark Harris <mhlk@....us>, Theodore Ts'o <tytso@....edu>,
	Jan Kara <jack@...e.cz>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6] e2fsck: Correct ext4 dates generated by old kernels.

On Nov 29, 2013, at 2:54 PM, David Turner <novalis@...alis.org> wrote:
> Is this version good, or should I make some more improvements?

The patch looks good to me (you could add a Reviewed-by: line for me if you want.

What you need to do now is to add a new test case or two to verify that this is
working correctly.  One way is to format a small binary image that has timestamps
of various "time eras".  Another way is to have the test script format a new image
each time and then use "debugfs -w -f" and the "sif" ("set_inode_field") command
to set the timestamps each time.  The only potential problem with "sif" is that
debugfs might internally "fix" the timestamps in the future and invalidate the
tests, but probably not.  The benefit of using debugfs+sif is that it is more clear
what the test is actually doing, and is easier to enhance in the future. 

In both cases, we want to verify that e.g. e2fsck fixes old timestamps and that
"debugfs -R 'stat testfile'" decodes the times correctly (atime, mtime, ctime,
and crtime). You'd need to set TZ in the test script so that the time zone in which
the test is run does not affect the results.  There is also the E2FSCK_TIME
environment variable that could be used to check e2fsck running beyond 2242.

Cheers, Andreas

> On Thu, 2013-11-14 at 16:06 -0500, David Turner wrote:
>> Not sure what the official subject line format is for revising only 
>> one of N patches, so I'm trying this one.  Let me now if it is wrong.
>> 
>> On Thu, 2013-11-14 at 02:15 -0800, Mark Harris wrote:
>>>> + * mis-encoded.
>>>> + */
>>>> +#define EXT4_EXTRA_NEGATIVE_DATE_CUTOFF 5 * (1ULL << 32)
>>> 
>>> Wouldn't 2242 be 0x200000000ULL, i.e. 2 * (1ULL << 32)?
>> 
>> Actually, it would be 2 * (1LL << 32), because we later use it in a
>> comparison with a signed value.  
>> 
>> --
>> Older kernels on 64-bit machines would incorrectly encode pre-1970
>> ext4 dates as post-2311 dates.  Detect and correct this (assuming the
>> current date is before 2242).
>> 
>> Signed-off-by: David Turner <novalis@...alis.org>
>> ---
>> e2fsck/pass1.c   | 41 +++++++++++++++++++++++++++++++++++++++++
>> e2fsck/problem.c |  4 ++++
>> e2fsck/problem.h |  4 ++++
>> 3 files changed, 49 insertions(+)
>> 
>> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
>> index ab23e42..e19855f 100644
>> --- a/e2fsck/pass1.c
>> +++ b/e2fsck/pass1.c
>> @@ -348,6 +348,24 @@ fix:
>> 				EXT2_INODE_SIZE(sb), "pass1");
>> }
>> 
>> +#define EXT4_EPOCH_BITS 2
>> +#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
>> +
>> +static int check_inode_extra_negative_epoch(__u32 xtime, __u32 extra) {
>> +	return (xtime & (1 << 31)) != 0 &&
>> +		(extra & EXT4_EPOCH_MASK) == EXT4_EPOCH_MASK;
>> +}
>> +
>> +#define CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, xtime) \
>> +	check_inode_extra_negative_epoch(inode->i_##xtime, \
>> +					 inode->i_##xtime##_extra)
>> +
>> +/* When today's date is earlier than 2242, we assume that atimes,
>> + * ctimes, and mtimes with years in the range 2310..2378 are actually
>> + * pre-1970 dates mis-encoded.
>> + */
>> +#define EXT4_EXTRA_NEGATIVE_DATE_CUTOFF 2 * (1LL << 32)
>> +
>> static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx)
>> {
>> 	struct ext2_super_block *sb = ctx->fs->super;
>> @@ -388,6 +406,29 @@ static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx)
>> 		/* it seems inode has an extended attribute(s) in body */
>> 		check_ea_in_inode(ctx, pctx);
>> 	}
>> +
>> +	/*
>> +	 * If the inode's extended atime (ctime, mtime) is stored in
>> +	 * the old, invalid format, repair it. 
>> +	 */
>> +	if (sizeof(time_t) > 4 && ctx->now < EXT4_EXTRA_NEGATIVE_DATE_CUTOFF &&
>> +	    (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, atime) ||
>> +	     CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, ctime) ||
>> +	     CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, mtime))) {
>> +
>> +		if (!fix_problem(ctx, PR_1_EA_TIME_OUT_OF_RANGE, pctx))
>> +			return;
>> +
>> +		if (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, atime))
>> +			inode->i_atime_extra &= ~EXT4_EPOCH_MASK;
>> +		if (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, ctime))
>> +			inode->i_ctime_extra &= ~EXT4_EPOCH_MASK;
>> +		if (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, mtime))
>> +			inode->i_mtime_extra &= ~EXT4_EPOCH_MASK;
>> +		e2fsck_write_inode_full(ctx, pctx->ino, pctx->inode,
>> +					EXT2_INODE_SIZE(sb), "pass1");
>> +	}
>> +
>> }
>> 
>> /*
>> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
>> index 897693a..b212d00 100644
>> --- a/e2fsck/problem.c
>> +++ b/e2fsck/problem.c
>> @@ -1018,6 +1018,10 @@ static struct e2fsck_problem problem_table[] = {
>> 	  N_("@i %i, end of extent exceeds allowed value\n\t(logical @b %c, physical @b %b, len %N)\n"),
>> 	  PROMPT_CLEAR, 0 },
>> 
>> +  /* Timestamp(s) on inode beyond 2310-04-04 are likely pre-1970 dates. */
>> +	{ PR_1_EA_TIME_OUT_OF_RANGE,
>> +		N_("Timestamp(s) on @i %i beyond 2310-04-04 are likely pre-1970 dates.\n"),
>> +		PROMPT_FIX | PR_PREEN_OK | PR_NO_OK, 0 },
>> 
>> 	/* Pass 1b errors */
>> 
>> diff --git a/e2fsck/problem.h b/e2fsck/problem.h
>> index ae1ed26..3710638 100644
>> --- a/e2fsck/problem.h
>> +++ b/e2fsck/problem.h
>> @@ -593,6 +593,10 @@ struct problem_context {
>> #define PR_1_EXTENT_INDEX_START_INVALID	0x01006D
>> 
>> #define PR_1_EXTENT_END_OUT_OF_BOUNDS	0x01006E
>> +
>> +/* Timestamp(s) on inode beyond 2310-04-04 are likely pre-1970 dates. */
>> +#define PR_1_EA_TIME_OUT_OF_RANGE	0x01006F
>> +
>> /*
>>  * Pass 1b errors
>>  */
> 
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists