[PATCH] ext4: Fix reading of extended tv_sec (bug 23732)

[PATCH] ext4: Fix reading of extended tv_sec (bug 23732)

[David Turner]

In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
the {a,c,m}time fields, deferring the year 2038 problem to the year
2446.  The representation (which this patch does not alter) is a bit
hackish, in that the most-significant bit is no longer (alone)
sufficient to indicate the sign.  That's because we're representing an
asymmetric range, with seven times as many positive values as
negative.

When decoding these extended fields, for times whose bottom 32 bits
would represent a negative number, sign extension causes the 64-bit
extended timestamp to be negative as well, which is not what's
intended.  This patch corrects that issue, so that the only negative
{a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
timestamps).

Signed-off-by: David Turner <novalis@novalis.org>
Reported-by: Mark Harris <mh8928@yahoo.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
---
 fs/ext4/ext4.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index af815ea..7b73c26 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -722,10 +722,15 @@ static inline __le32 ext4_encode_extra_time(struct timespec *time)
 
 static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
 {
-       if (sizeof(time->tv_sec) > 4)
-	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
-			       << 32;
-       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
+	if (sizeof(time->tv_sec) > 4) {
+		u64 extra_bits = (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK);
+		if (time->tv_sec > 0 || extra_bits != EXT4_EPOCH_MASK) {
+			time->tv_sec &= 0xFFFFFFFF;
+			time->tv_sec |= extra_bits << 32;
+		}
+		time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
+			EXT4_EPOCH_BITS;
+	}
 }
 
 #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
-- 
1.8.1.2

Re: [PATCH] ext4: Fix reading of extended tv_sec (bug 23732)

[Jan Kara]

On Thu 07-11-13 02:16:30, David Turner wrote:
> In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
> the {a,c,m}time fields, deferring the year 2038 problem to the year
> 2446.  The representation (which this patch does not alter) is a bit
> hackish, in that the most-significant bit is no longer (alone)
> sufficient to indicate the sign.  That's because we're representing an
> asymmetric range, with seven times as many positive values as
> negative.
> 
> When decoding these extended fields, for times whose bottom 32 bits
> would represent a negative number, sign extension causes the 64-bit
> extended timestamp to be negative as well, which is not what's
> intended.  This patch corrects that issue, so that the only negative
> {a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
> timestamps).
> 
> Signed-off-by: David Turner <novalis@novalis.org>
> Reported-by: Mark Harris <mh8928@yahoo.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
> ---
>  fs/ext4/ext4.h | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index af815ea..7b73c26 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -722,10 +722,15 @@ static inline __le32 ext4_encode_extra_time(struct timespec *time)
>  
>  static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
>  {
> -       if (sizeof(time->tv_sec) > 4)
> -	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> -			       << 32;
> -       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
> +	if (sizeof(time->tv_sec) > 4) {
> +		u64 extra_bits = (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK);
  The extra cast to (__u64) looks useless here.

> +		if (time->tv_sec > 0 || extra_bits != EXT4_EPOCH_MASK) {
> +			time->tv_sec &= 0xFFFFFFFF;
> +			time->tv_sec |= extra_bits << 32;
> +		}
> +		time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
> +			EXT4_EPOCH_BITS;
> +	}
>  }
  So I'm somewhat wondering: Previously we decoded tv_nsec regardless of
tv_sec size. After your patch we do it only if sizeof(time->tv_sec) > 4. Is
this an intended change? Why is it OK?

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH v2] ext4: Fix reading of extended tv_sec (bug 23732)

[David Turner]

On Thu, 2013-11-07 at 17:03 +0100, Jan Kara wrote:
>   So I'm somewhat wondering: Previously we decoded tv_nsec regardless of
> tv_sec size. After your patch we do it only if sizeof(time->tv_sec) > 4. Is
> this an intended change? Why is it OK?

This is an error.  Here is a corrected version of the patch. 


--

In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
the {a,c,m}time fields, deferring the year 2038 problem to the year
2446.  The representation (which this patch does not alter) is a bit
hackish, in that the most-significant bit is no longer (alone)
sufficient to indicate the sign.  That's because we're representing an
asymmetric range, with seven times as many positive values as
negative.

When decoding these extended fields, for times whose bottom 32 bits
would represent a negative number, sign extension causes the 64-bit
extended timestamp to be negative as well, which is not what's
intended.  This patch corrects that issue, so that the only negative
{a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
timestamps).

Signed-off-by: David Turner <novalis@novalis.org>
Reported-by: Mark Harris <mh8928@yahoo.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
---
 fs/ext4/ext4.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index af815ea..3c2d0b3 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -722,10 +722,15 @@ static inline __le32 ext4_encode_extra_time(struct timespec *time)
 
 static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
 {
-       if (sizeof(time->tv_sec) > 4)
-	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
-			       << 32;
-       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
+	if (sizeof(time->tv_sec) > 4) {
+		u64 extra_bits = (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK);
+		if (time->tv_sec > 0 || extra_bits != EXT4_EPOCH_MASK) {
+			time->tv_sec &= 0xFFFFFFFF;
+			time->tv_sec |= extra_bits << 32;
+		}
+	}
+	time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
+		EXT4_EPOCH_BITS;
 }
 
 #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
-- 
1.8.1.2

Re: [PATCH v2] ext4: Fix reading of extended tv_sec (bug 23732)

[Jan Kara]

On Thu 07-11-13 17:54:24, David Turner wrote:
> On Thu, 2013-11-07 at 17:03 +0100, Jan Kara wrote:
> >   So I'm somewhat wondering: Previously we decoded tv_nsec regardless of
> > tv_sec size. After your patch we do it only if sizeof(time->tv_sec) > 4. Is
> > this an intended change? Why is it OK?
> 
> This is an error.  Here is a corrected version of the patch. 
> 
> 
> --
> 
> In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
> the {a,c,m}time fields, deferring the year 2038 problem to the year
> 2446.  The representation (which this patch does not alter) is a bit
> hackish, in that the most-significant bit is no longer (alone)
> sufficient to indicate the sign.  That's because we're representing an
> asymmetric range, with seven times as many positive values as
> negative.
> 
> When decoding these extended fields, for times whose bottom 32 bits
> would represent a negative number, sign extension causes the 64-bit
> extended timestamp to be negative as well, which is not what's
> intended.  This patch corrects that issue, so that the only negative
> {a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
> timestamps).
> 
> Signed-off-by: David Turner <novalis@novalis.org>
> Reported-by: Mark Harris <mh8928@yahoo.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
> ---
>  fs/ext4/ext4.h | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index af815ea..3c2d0b3 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -722,10 +722,15 @@ static inline __le32 ext4_encode_extra_time(struct timespec *time)
>  
>  static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
>  {
> -       if (sizeof(time->tv_sec) > 4)
> -	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> -			       << 32;
> -       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
> +	if (sizeof(time->tv_sec) > 4) {
> +		u64 extra_bits = (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK);
                                  ^^^^
Still unnecessary type cast here (but that's a cosmetic issue).

Otherwise the patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> +		if (time->tv_sec > 0 || extra_bits != EXT4_EPOCH_MASK) {
> +			time->tv_sec &= 0xFFFFFFFF;
> +			time->tv_sec |= extra_bits << 32;
> +		}
> +	}
> +	time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
> +		EXT4_EPOCH_BITS;
>  }
>  
>  #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
> -- 
> 1.8.1.2
> 
> 
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH v3] ext4: Fix reading of extended tv_sec (bug 23732)

[David Turner]

On Fri, 2013-11-08 at 00:14 +0100, Jan Kara wrote:
> Still unnecessary type cast here (but that's a cosmetic issue).
...
> Otherwise the patch looks good. You can add:
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks.  A version with this correction and the reviewed-by follows.

--
In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
the {a,c,m}time fields, deferring the year 2038 problem to the year
2446.  The representation (which this patch does not alter) is a bit
hackish, in that the most-significant bit is no longer (alone)
sufficient to indicate the sign.  That's because we're representing an
asymmetric range, with seven times as many positive values as
negative.

When decoding these extended fields, for times whose bottom 32 bits
would represent a negative number, sign extension causes the 64-bit
extended timestamp to be negative as well, which is not what's
intended.  This patch corrects that issue, so that the only negative
{a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
timestamps).

Signed-off-by: David Turner <novalis@novalis.org>
Reported-by: Mark Harris <mh8928@yahoo.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/ext4.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index af815ea..3c2d0b3 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -722,10 +722,15 @@ static inline __le32 ext4_encode_extra_time(struct timespec *time)
 
 static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
 {
-       if (sizeof(time->tv_sec) > 4)
-	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
-			       << 32;
-       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
+	if (sizeof(time->tv_sec) > 4) {
+		u64 extra_bits = le32_to_cpu(extra) & EXT4_EPOCH_MASK;
+		if (time->tv_sec > 0 || extra_bits != EXT4_EPOCH_MASK) {
+			time->tv_sec &= 0xFFFFFFFF;
+			time->tv_sec |= extra_bits << 32;
+		}
+	}
+	time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
+		EXT4_EPOCH_BITS;
 }
 
 #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
-- 
1.8.1.2

Re: [PATCH v3] ext4: Fix reading of extended tv_sec (bug 23732)

[Theodore Ts'o]

On Thu, Nov 07, 2013 at 06:26:47PM -0500, David Turner wrote:
> In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
> the {a,c,m}time fields, deferring the year 2038 problem to the year
> 2446.  The representation (which this patch does not alter) is a bit
> hackish, in that the most-significant bit is no longer (alone)
> sufficient to indicate the sign.  That's because we're representing an
> asymmetric range, with seven times as many positive values as
> negative.
> 
> When decoding these extended fields, for times whose bottom 32 bits
> would represent a negative number, sign extension causes the 64-bit
> extended timestamp to be negative as well, which is not what's
> intended.  This patch corrects that issue, so that the only negative
> {a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
> timestamps).
> 
> Signed-off-by: David Turner <novalis@novalis.org>
> Reported-by: Mark Harris <mh8928@yahoo.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, applied.

					- Ted

Re: [PATCH v3] ext4: Fix reading of extended tv_sec (bug 23732)

[Andreas Dilger]

On Nov 7, 2013, at 4:26 PM, David Turner <novalis@novalis.org> wrote:
> On Fri, 2013-11-08 at 00:14 +0100, Jan Kara wrote:
>> Still unnecessary type cast here (but that's a cosmetic issue).
> ...
>> Otherwise the patch looks good. You can add:
>> Reviewed-by: Jan Kara <jack@suse.cz>
> 
> Thanks.  A version with this correction and the reviewed-by follows.

Thanks for working on this.  It was (seriously) in my list of things to
get done, but I figured I still had a few years to work on it...

My (unfinished) version of this patch had a nice comment at ext4_encode_time()
that explained the encoding that is being used very clearly:

/*
 * We need is an encoding that preserves the times for extra epoch "00":
 *
 * extra  msb of                         adjust for signed
 * epoch  32-bit                         32-bit tv_sec to
 * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
 * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
 * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
 * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
 * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
 * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
 * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
 * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
 * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
 */

It seems that your version of the patch seems to use a different encoding.  Not
that this is a problem, per-se, since my patch wasn’t in use anywhere, but it
would be nice to have a similarly clear explanation of what the mapping is so
that it can be clearly understood.

My ext4_{encode,decode}_extra_time() used add/subtract instead of mask/OR ops,
which changed the on-disk format for times beyond 2038, but those were already
broken, and presumably not in use by anyone yet.  However, it seemed to me this
was easier to produce the correct results.  Have you tested your patch with
a variety of timestamps to verify its correctness?  It looks to me like you
have mapped the 1901-1969 range onto 0x3 for the epoch bits, instead of the
(IMHO) natural 0x0 bits. The critical time ranges are listed above.

Cheers, Andreas

> --
> In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
> the {a,c,m}time fields, deferring the year 2038 problem to the year
> 2446.  The representation (which this patch does not alter) is a bit
> hackish, in that the most-significant bit is no longer (alone)
> sufficient to indicate the sign.  That's because we're representing an
> asymmetric range, with seven times as many positive values as
> negative.
> 
> When decoding these extended fields, for times whose bottom 32 bits
> would represent a negative number, sign extension causes the 64-bit
> extended timestamp to be negative as well, which is not what's
> intended.  This patch corrects that issue, so that the only negative
> {a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
> timestamps).
> 
> Signed-off-by: David Turner <novalis@novalis.org>
> Reported-by: Mark Harris <mh8928@yahoo.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
> fs/ext4/ext4.h | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index af815ea..3c2d0b3 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -722,10 +722,15 @@ static inline __le32 ext4_encode_extra_time(struct timespec *time)
> 
> static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
> {
> -       if (sizeof(time->tv_sec) > 4)
> -	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> -			       << 32;
> -       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
> +	if (sizeof(time->tv_sec) > 4) {
> +		u64 extra_bits = le32_to_cpu(extra) & EXT4_EPOCH_MASK;
> +		if (time->tv_sec > 0 || extra_bits != EXT4_EPOCH_MASK) {
> +			time->tv_sec &= 0xFFFFFFFF;
> +			time->tv_sec |= extra_bits << 32;
> +		}
> +	}
> +	time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
> +		EXT4_EPOCH_BITS;
> }
> 
> #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
> -- 
> 1.8.1.2
> 
> 
> 


Cheers, Andreas

[PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[David Turner]

On Fri, 2013-11-08 at 14:37 -0700, Andreas Dilger wrote:
> On Nov 7, 2013, at 4:26 PM, David Turner <novalis@novalis.org> wrote:
> > On Fri, 2013-11-08 at 00:14 +0100, Jan Kara wrote:
> >> Still unnecessary type cast here (but that's a cosmetic issue).
> > ...
> >> Otherwise the patch looks good. You can add:
> >> Reviewed-by: Jan Kara <jack@suse.cz>
> > 
> > Thanks.  A version with this correction and the reviewed-by follows.
> 
> Thanks for working on this.  It was (seriously) in my list of things to
> get done, but I figured I still had a few years to work on it...
> 
> My (unfinished) version of this patch had a nice comment at ext4_encode_time()
> that explained the encoding that is being used very clearly:
> 
> /*
>  * We need is an encoding that preserves the times for extra epoch "00":
>  *
>  * extra  msb of                         adjust for signed
>  * epoch  32-bit                         32-bit tv_sec to
>  * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
>  * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
>  * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
>  * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
>  * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
>  * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
>  * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
>  * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
>  * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
>  */
> 
> It seems that your version of the patch seems to use a different encoding.  Not
> that this is a problem, per-se, since my patch wasn’t in use anywhere, but it
> would be nice to have a similarly clear explanation of what the mapping is so
> that it can be clearly understood.

I have included a patch with an explanation (the patch is against
tytso/dev -- I hope that's the correct place).  

> My ext4_{encode,decode}_extra_time() used add/subtract instead of mask/OR ops,
> which changed the on-disk format for times beyond 2038, but those were already
> broken, and presumably not in use by anyone yet. 

They were actually correct according to my patch's encoding (that is, my
patch used the existing encoding).  The existing encoding was written
correctly, but read wrongly.  As you say, this should not matter, since
nobody should be writing these timestamps, but I assumed that the
existing encoding had happened for a reason, and wanted to make the
minimal change.

If you believe it is important, I would be happy to change it.

>  However, it seemed to me this
> was easier to produce the correct results.  Have you tested your patch with
> a variety of timestamps to verify its correctness? 

I tested by using touch -d to create one file in each year between 1902
and 2446.  Then I unmounted and remounted the FS, and did ls -l and
manually verified that each file's date matched its name.

> It looks to me like you
> have mapped the 1901-1969 range onto 0x3 for the epoch bits, instead 
> of the (IMHO) natural 0x0 bits. The critical time ranges are listed 
> above.

I think the idea of this is that it is the bottom 34 bits of the 64-bit
signed time.  However, it occurs to me that this relies on a two's
complement machine.  Even though the C standard does not guarantee this,
I believe the kernel requires it, so that's probably OK.

Patch follows:

--
Add a comment explaining the encoding of ext4's extra {a,c,m}time
bits.

Signed-off-by: David Turner <novalis@novalis.org>
---
 fs/ext4/ext4.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 121da383..ab69f14 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -713,6 +713,24 @@ struct move_extent {
 	  sizeof((ext4_inode)->field))			\
 	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
 	    (einode)->i_extra_isize))			\
+/*
+ * We use the bottom 34 bits of the signed 64-bit time value, with
+ * the top two of these bits in the bottom of extra.  This leads
+ * to a slightly odd encoding, which works like this:
+ *
+ * extra  msb of
+ * epoch  32-bit
+ * bits   time    decoded 64-bit tv_sec   valid time range
+ * 0 0    0    0x000000000..0x07fffffff  1970-01-01..2038-01-19
+ * 0 0    1    0x080000000..0x0ffffffff  2038-01-19..2106-02-07
+ * 0 1    0    0x100000000..0x17fffffff  2106-02-07..2174-02-25
+ * 0 1    1    0x180000000..0x1ffffffff  2174-02-25..2242-03-16
+ * 1 0    0    0x200000000..0x27fffffff  2242-03-16..2310-04-04
+ * 1 0    1    0x280000000..0x2ffffffff  2310-04-04..2378-04-22
+ * 1 1    0    0x300000000..0x37fffffff  2378-04-22..2446-05-10
+
+ * 1 1    1    -0x80000000..-0x00000001  1901-12-13..1969-12-31
+ */
 
 static inline __le32 ext4_encode_extra_time(struct timespec *time)
 {
-- 
1.8.1.2

[PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[David Turner]

On Fri, 2013-11-08 at 14:37 -0700, Andreas Dilger wrote:
> On Nov 7, 2013, at 4:26 PM, David Turner <novalis@novalis.org> wrote:
> > On Fri, 2013-11-08 at 00:14 +0100, Jan Kara wrote:
> >> Still unnecessary type cast here (but that's a cosmetic issue).
> > ...
> >> Otherwise the patch looks good. You can add:
> >> Reviewed-by: Jan Kara <jack@suse.cz>
> > 
> > Thanks.  A version with this correction and the reviewed-by follows.
> 
> Thanks for working on this.  It was (seriously) in my list of things to
> get done, but I figured I still had a few years to work on it...
> 
> My (unfinished) version of this patch had a nice comment at ext4_encode_time()
> that explained the encoding that is being used very clearly:
> 
> /*
>  * We need is an encoding that preserves the times for extra epoch "00":
>  *
>  * extra  msb of                         adjust for signed
>  * epoch  32-bit                         32-bit tv_sec to
>  * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
>  * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
>  * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
>  * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
>  * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
>  * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
>  * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
>  * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
>  * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
>  */
> 
> It seems that your version of the patch seems to use a different encoding.  Not
> that this is a problem, per-se, since my patch wasn’t in use anywhere, but it
> would be nice to have a similarly clear explanation of what the mapping is so
> that it can be clearly understood.

I have included a patch with an explanation (the patch is against
tytso/dev -- I hope that's the correct place).  

> My ext4_{encode,decode}_extra_time() used add/subtract instead of mask/OR ops,
> which changed the on-disk format for times beyond 2038, but those were already
> broken, and presumably not in use by anyone yet. 

They were actually correct according to my patch's encoding (that is, my
patch used the existing encoding).  The existing encoding was written
correctly, but read wrongly.  As you say, this should not matter, since
nobody should be writing these timestamps, but I assumed that the
existing encoding had happened for a reason, and wanted to make the
minimal change.

If you believe it is important, I would be happy to change it.

>  However, it seemed to me this
> was easier to produce the correct results.  Have you tested your patch with
> a variety of timestamps to verify its correctness? 

I tested by using touch -d to create one file in each year between 1902
and 2446.  Then I unmounted and remounted the FS, and did ls -l and
manually verified that each file's date matched its name.

> It looks to me like you
> have mapped the 1901-1969 range onto 0x3 for the epoch bits, instead 
> of the (IMHO) natural 0x0 bits. The critical time ranges are listed 
> above.

I think the idea of this is that it is the bottom 34 bits of the 64-bit
signed time.  However, it occurs to me that this relies on a two's
complement machine.  Even though the C standard does not guarantee this,
I believe the kernel requires it, so that's probably OK.

Patch follows:

--
Add a comment explaining the encoding of ext4's extra {a,c,m}time
bits.

Signed-off-by: David Turner <novalis@novalis.org>
---
 fs/ext4/ext4.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 121da383..ab69f14 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -713,6 +713,24 @@ struct move_extent {
 	  sizeof((ext4_inode)->field))			\
 	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
 	    (einode)->i_extra_isize))			\
+/*
+ * We use the bottom 34 bits of the signed 64-bit time value, with
+ * the top two of these bits in the bottom of extra.  This leads
+ * to a slightly odd encoding, which works like this:
+ *
+ * extra  msb of
+ * epoch  32-bit
+ * bits   time    decoded 64-bit tv_sec   valid time range
+ * 0 0    0    0x000000000..0x07fffffff  1970-01-01..2038-01-19
+ * 0 0    1    0x080000000..0x0ffffffff  2038-01-19..2106-02-07
+ * 0 1    0    0x100000000..0x17fffffff  2106-02-07..2174-02-25
+ * 0 1    1    0x180000000..0x1ffffffff  2174-02-25..2242-03-16
+ * 1 0    0    0x200000000..0x27fffffff  2242-03-16..2310-04-04
+ * 1 0    1    0x280000000..0x2ffffffff  2310-04-04..2378-04-22
+ * 1 1    0    0x300000000..0x37fffffff  2378-04-22..2446-05-10
+
+ * 1 1    1    -0x80000000..-0x00000001  1901-12-13..1969-12-31
+ */
 
 static inline __le32 ext4_encode_extra_time(struct timespec *time)
 {
-- 
1.8.1.2



--
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

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[Mark Harris]

On 8 November 2013 23:19, David Turner <novalis@novalis.org> wrote:
>
> On Fri, 2013-11-08 at 14:37 -0700, Andreas Dilger wrote:
> > On Nov 7, 2013, at 4:26 PM, David Turner <novalis@novalis.org> wrote:
> > > On Fri, 2013-11-08 at 00:14 +0100, Jan Kara wrote:
> > >> Still unnecessary type cast here (but that's a cosmetic issue).
> > > ...
> > >> Otherwise the patch looks good. You can add:
> > >> Reviewed-by: Jan Kara <jack@suse.cz>
> > >
> > > Thanks.  A version with this correction and the reviewed-by follows.
> >
> > Thanks for working on this.  It was (seriously) in my list of things to
> > get done, but I figured I still had a few years to work on it...
> >
> > My (unfinished) version of this patch had a nice comment at ext4_encode_time()
> > that explained the encoding that is being used very clearly:
> >
> > /*
> >  * We need is an encoding that preserves the times for extra epoch "00":
> >  *
> >  * extra  msb of                         adjust for signed
> >  * epoch  32-bit                         32-bit tv_sec to
> >  * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
> >  * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
> >  * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
> >  * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
> >  * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
> >  * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
> >  * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
> >  * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
> >  * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
> >  */
> >
> > It seems that your version of the patch seems to use a different encoding.  Not
> > that this is a problem, per-se, since my patch wasn’t in use anywhere, but it
> > would be nice to have a similarly clear explanation of what the mapping is so
> > that it can be clearly understood.
>
> I have included a patch with an explanation (the patch is against
> tytso/dev -- I hope that's the correct place).
>
> > My ext4_{encode,decode}_extra_time() used add/subtract instead of mask/OR ops,
> > which changed the on-disk format for times beyond 2038, but those were already
> > broken, and presumably not in use by anyone yet.
>
> They were actually correct according to my patch's encoding (that is, my
> patch used the existing encoding).  The existing encoding was written
> correctly, but read wrongly.  As you say, this should not matter, since
> nobody should be writing these timestamps, but I assumed that the
> existing encoding had happened for a reason, and wanted to make the
> minimal change.
>
> If you believe it is important, I would be happy to change it.


The problem with the existing encoding is that pre-1970 dates are
encoded with extra bits 1,1 in 64-bit kernels with ext4, but on 32-bit
kernels and inodes that were originally written as ext3 the extra bits
will be 0,0.  Currently, both are decoded as pre-1970 dates.

With your patch, only the 1,1 format used by 64-bit ext4 will decode
as a pre-1970 date.  Dates previously written by ext3 or a 32-bit
kernel will no longer be decoded as expected.  Also the patch does
not update ext4_encode_extra_time to use this format for pre-1970
dates in 32-bit mode.

Possible solutions were discussed here, but no decision was made:
http://thread.gmane.org/gmane.comp.file-systems.ext4/26087/focus=26406


>
>
> >  However, it seemed to me this
> > was easier to produce the correct results.  Have you tested your patch with
> > a variety of timestamps to verify its correctness?
>
> I tested by using touch -d to create one file in each year between 1902
> and 2446.  Then I unmounted and remounted the FS, and did ls -l and
> manually verified that each file's date matched its name.
>
> > It looks to me like you
> > have mapped the 1901-1969 range onto 0x3 for the epoch bits, instead
> > of the (IMHO) natural 0x0 bits. The critical time ranges are listed
> > above.
>
> I think the idea of this is that it is the bottom 34 bits of the 64-bit
> signed time.  However, it occurs to me that this relies on a two's
> complement machine.  Even though the C standard does not guarantee this,
> I believe the kernel requires it, so that's probably OK.

 - Mark

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[Mark Harris]

On 8 November 2013 23:19, David Turner <novalis@novalis.org> wrote:
>
> On Fri, 2013-11-08 at 14:37 -0700, Andreas Dilger wrote:
> > On Nov 7, 2013, at 4:26 PM, David Turner <novalis@novalis.org> wrote:
> > > On Fri, 2013-11-08 at 00:14 +0100, Jan Kara wrote:
> > >> Still unnecessary type cast here (but that's a cosmetic issue).
> > > ...
> > >> Otherwise the patch looks good. You can add:
> > >> Reviewed-by: Jan Kara <jack@suse.cz>
> > >
> > > Thanks.  A version with this correction and the reviewed-by follows.
> >
> > Thanks for working on this.  It was (seriously) in my list of things to
> > get done, but I figured I still had a few years to work on it...
> >
> > My (unfinished) version of this patch had a nice comment at ext4_encode_time()
> > that explained the encoding that is being used very clearly:
> >
> > /*
> >  * We need is an encoding that preserves the times for extra epoch "00":
> >  *
> >  * extra  msb of                         adjust for signed
> >  * epoch  32-bit                         32-bit tv_sec to
> >  * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
> >  * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
> >  * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
> >  * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
> >  * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
> >  * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
> >  * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
> >  * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
> >  * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
> >  */
> >
> > It seems that your version of the patch seems to use a different encoding.  Not
> > that this is a problem, per-se, since my patch wasn’t in use anywhere, but it
> > would be nice to have a similarly clear explanation of what the mapping is so
> > that it can be clearly understood.
>
> I have included a patch with an explanation (the patch is against
> tytso/dev -- I hope that's the correct place).
>
> > My ext4_{encode,decode}_extra_time() used add/subtract instead of mask/OR ops,
> > which changed the on-disk format for times beyond 2038, but those were already
> > broken, and presumably not in use by anyone yet.
>
> They were actually correct according to my patch's encoding (that is, my
> patch used the existing encoding).  The existing encoding was written
> correctly, but read wrongly.  As you say, this should not matter, since
> nobody should be writing these timestamps, but I assumed that the
> existing encoding had happened for a reason, and wanted to make the
> minimal change.
>
> If you believe it is important, I would be happy to change it.


The problem with the existing encoding is that pre-1970 dates are
encoded with extra bits 1,1 in 64-bit kernels with ext4, but on 32-bit
kernels and inodes that were originally written as ext3 the extra bits
will be 0,0.  Currently, both are decoded as pre-1970 dates.

With your patch, only the 1,1 format used by 64-bit ext4 will decode
as a pre-1970 date.  Dates previously written by ext3 or a 32-bit
kernel will no longer be decoded as expected.  Also the patch does
not update ext4_encode_extra_time to use this format for pre-1970
dates in 32-bit mode.

Possible solutions were discussed here, but no decision was made:
http://thread.gmane.org/gmane.comp.file-systems.ext4/26087/focus=26406


>
>
> >  However, it seemed to me this
> > was easier to produce the correct results.  Have you tested your patch with
> > a variety of timestamps to verify its correctness?
>
> I tested by using touch -d to create one file in each year between 1902
> and 2446.  Then I unmounted and remounted the FS, and did ls -l and
> manually verified that each file's date matched its name.
>
> > It looks to me like you
> > have mapped the 1901-1969 range onto 0x3 for the epoch bits, instead
> > of the (IMHO) natural 0x0 bits. The critical time ranges are listed
> > above.
>
> I think the idea of this is that it is the bottom 34 bits of the 64-bit
> signed time.  However, it occurs to me that this relies on a two's
> complement machine.  Even though the C standard does not guarantee this,
> I believe the kernel requires it, so that's probably OK.

 - Mark
--
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

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[David Turner]

On Sat, 2013-11-09 at 15:51 -0800, Mark Harris wrote:
> 
> The problem with the existing encoding is that pre-1970 dates are
> encoded with extra bits 1,1 in 64-bit kernels with ext4, but on 32-bit
> kernels and inodes that were originally written as ext3 the extra bits
> will be 0,0.  Currently, both are decoded as pre-1970 dates.
> 
> With your patch, only the 1,1 format used by 64-bit ext4 will decode
> as a pre-1970 date.  Dates previously written by ext3 or a 32-bit
> kernel will no longer be decoded as expected.  Also the patch does
> not update ext4_encode_extra_time to use this format for pre-1970
> dates in 32-bit mode.

You're right -- I missed the complexity here.

> Possible solutions were discussed here, but no decision was made:
> http://thread.gmane.org/gmane.comp.file-systems.ext4/26087/focus=26406

To summarize, the previous discussion offered four possible solutions,
of which two were thought good:

b. Use Andreas's encoding, which is incompatible with pre-1970 files
written on 64-bit systems.

c. Use an encoding which is compatible with all pre-1970 files, but
incompatible with 64-bit post-2038 files, and which encodes a smaller
range of time and is more complicated. 

-------
I don't care about currently-existing post-2038 files, because I believe
that nobody has a valid reason to have such files.  However, I do
believe that pre-1970 files are probably important to someone.

Despite this, I prefer option (b), because I think the simplicity is
valuable, and because I hate to give up date ranges (even ones that I
think we'll "never" need). Option (b) is not actually lossy, because we
could correct pre-1970 files with e2fsck; under Andreas's encoding,
their dates would be in the far future (and thus cannot be legitimate).

Would a patch that does (b) be accepted?  I would accompany it with a
patch to e2fsck (which I assume would also go to the ext4 developers
mailing list?).

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[Theodore Ts'o]

On Sun, Nov 10, 2013 at 02:56:54AM -0500, David Turner wrote:
> b. Use Andreas's encoding, which is incompatible with pre-1970 files
> written on 64-bit systems.
>
> I don't care about currently-existing post-2038 files, because I believe
> that nobody has a valid reason to have such files.  However, I do
> believe that pre-1970 files are probably important to someone.
> 
> Despite this, I prefer option (b), because I think the simplicity is
> valuable, and because I hate to give up date ranges (even ones that I
> think we'll "never" need). Option (b) is not actually lossy, because we
> could correct pre-1970 files with e2fsck; under Andreas's encoding,
> their dates would be in the far future (and thus cannot be legitimate).
> 
> Would a patch that does (b) be accepted?  I would accompany it with a
> patch to e2fsck (which I assume would also go to the ext4 developers
> mailing list?).

I agree, I think this is the best way to go.  I'm going to drop your
earlier patch, and wait for an updated patch from you.  It may miss
this merge window, but as Andreas has pointed out, we still have a few
years to get this right.  :-)

Thanks!!

					- Ted

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2004 bytes --]

On Nov 11, 2013, at 5:30 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Sun, Nov 10, 2013 at 02:56:54AM -0500, David Turner wrote:
>> b. Use Andreas's encoding, which is incompatible with pre-1970 files
>> written on 64-bit systems.
>> 
>> I don't care about currently-existing post-2038 files, because I believe
>> that nobody has a valid reason to have such files.  However, I do
>> believe that pre-1970 files are probably important to someone.
>> 
>> Despite this, I prefer option (b), because I think the simplicity is
>> valuable, and because I hate to give up date ranges (even ones that I
>> think we'll "never" need). Option (b) is not actually lossy, because we
>> could correct pre-1970 files with e2fsck; under Andreas's encoding,
>> their dates would be in the far future (and thus cannot be legitimate).
>> 
>> Would a patch that does (b) be accepted?  I would accompany it with a
>> patch to e2fsck (which I assume would also go to the ext4 developers
>> mailing list?).
> 
> I agree, I think this is the best way to go.  I'm going to drop your
> earlier patch, and wait for an updated patch from you.  It may miss
> this merge window, but as Andreas has pointed out, we still have a few
> years to get this right.  :-)

Since this change would immediately break files encoded with pre-1970 dates
on ext4 filesystems, should there be a transition period where both pre-1970
dates (with extra epoch bits == 0x3 in the current encoding) and post-2378
(with extra epoch bits == 0x3 in the new encoding) are decoded as being
pre-1970?  That could be conditional until some release in the future (e.g.
>= Linux 4.20, at least 5 years away) to give folks a chance to run the new
e2fsck to fix up those files.

Are there really any ext4 filesystems that have files with valid dates so old?
I don’t want to break anyone’s data, but if this extra complexity is completely
pointless then I’m also fine with this minor risk of breakage.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[Darrick J. Wong]

On Mon, Nov 11, 2013 at 07:30:18PM -0500, Theodore Ts'o wrote:
> On Sun, Nov 10, 2013 at 02:56:54AM -0500, David Turner wrote:
> > b. Use Andreas's encoding, which is incompatible with pre-1970 files
> > written on 64-bit systems.
> >
> > I don't care about currently-existing post-2038 files, because I believe
> > that nobody has a valid reason to have such files.  However, I do
> > believe that pre-1970 files are probably important to someone.
> > 
> > Despite this, I prefer option (b), because I think the simplicity is
> > valuable, and because I hate to give up date ranges (even ones that I
> > think we'll "never" need). Option (b) is not actually lossy, because we
> > could correct pre-1970 files with e2fsck; under Andreas's encoding,
> > their dates would be in the far future (and thus cannot be legitimate).
> > 
> > Would a patch that does (b) be accepted?  I would accompany it with a
> > patch to e2fsck (which I assume would also go to the ext4 developers
> > mailing list?).
> 
> I agree, I think this is the best way to go.  I'm going to drop your
> earlier patch, and wait for an updated patch from you.  It may miss
> this merge window, but as Andreas has pointed out, we still have a few
> years to get this right.  :-)

Just to be clear... we're going with Andreas' fix, wherein

time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;

becomes:

time->tv_sec += (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;

"or" becomes "plus"?  So I can update fuse2fs.

Also, can someone proofread [1] and make sure it's correct?

--D

[1] https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout#Inode_Timestamps
> 
> Thanks!!
> 
> 					- 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

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[David Turner]

On Tue, 2013-11-12 at 15:03 -0800, Darrick J. Wong wrote:
> On Mon, Nov 11, 2013 at 07:30:18PM -0500, Theodore Ts'o wrote:
> > On Sun, Nov 10, 2013 at 02:56:54AM -0500, David Turner wrote:
> > > b. Use Andreas's encoding, which is incompatible with pre-1970 files
> > > written on 64-bit systems.
> > >
> > > I don't care about currently-existing post-2038 files, because I believe
> > > that nobody has a valid reason to have such files.  However, I do
> > > believe that pre-1970 files are probably important to someone.
> > > 
> > > Despite this, I prefer option (b), because I think the simplicity is
> > > valuable, and because I hate to give up date ranges (even ones that I
> > > think we'll "never" need). Option (b) is not actually lossy, because we
> > > could correct pre-1970 files with e2fsck; under Andreas's encoding,
> > > their dates would be in the far future (and thus cannot be legitimate).
> > > 
> > > Would a patch that does (b) be accepted?  I would accompany it with a
> > > patch to e2fsck (which I assume would also go to the ext4 developers
> > > mailing list?).
> > 
> > I agree, I think this is the best way to go.  I'm going to drop your
> > earlier patch, and wait for an updated patch from you.  It may miss
> > this merge window, but as Andreas has pointed out, we still have a few
> > years to get this right.  :-)
> 
> Just to be clear... we're going with Andreas' fix, wherein
> 
> time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;
> 
> becomes:
> 
> time->tv_sec += (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;
> 
> "or" becomes "plus"?  So I can update fuse2fs.

Yes, but with a kernel-version-dependent change, which is something like
this:
#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,20,0)
		time->tv_sec += (u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;
#else
		u64 extra_bits = le32_to_cpu(extra) & EXT4_EPOCH_MASK;
		if (extra_bits == 3)
			extra_bits = 0;
		time->tv_sec += extra_bits << 32;
#endif

> Also, can someone proofread [1] and make sure it's correct?

It's not quite right.  I've requested an account so that I can correct
it.

[PATCH v4 1/2] ext4: Fix handling of extended tv_sec (bug 23732)

[David Turner]

In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
the {a,c,m}time fields, deferring the year 2038 problem to the year
2446.

When decoding these extended fields, for times whose bottom 32 bits
would represent a negative number, sign extension causes the 64-bit
extended timestamp to be negative as well, which is not what's
intended.  This patch corrects that issue, so that the only negative
{a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
timestamps).

Some older kernels might have written pre-1970 dates with 1,1 in the
extra bits.  This patch treats those incorrectly-encoded dates as
pre-1970, instead of post-2311, until kernel 4.20 is released.
Hopefully by then e2fsck will have fixed up the bad data.

Signed-off-by: David Turner <novalis@novalis.org>
Reported-by: Mark Harris <mh8928@yahoo.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
---
 fs/ext4/ext4.h | 61 +++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 18aa56b..7d5e019 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -26,6 +26,7 @@
 #include <linux/seqlock.h>
 #include <linux/mutex.h>
 #include <linux/timer.h>
+#include <linux/version.h>
 #include <linux/wait.h>
 #include <linux/blockgroup_lock.h>
 #include <linux/percpu_counter.h>
@@ -713,38 +714,54 @@ struct move_extent {
 	  sizeof((ext4_inode)->field))			\
 	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
 	    (einode)->i_extra_isize))			\
+
 /*
- * We use the bottom 34 bits of the signed 64-bit time value, with
- * the top two of these bits in the bottom of extra.  This leads
- * to a slightly odd encoding, which works like this:
+ * We need is an encoding that preserves the times for extra epoch "00":
  *
- * extra  msb of
- * epoch  32-bit
- * bits   time    decoded 64-bit tv_sec   valid time range
- * 0 0    0    0x000000000..0x07fffffff  1970-01-01..2038-01-19
- * 0 0    1    0x080000000..0x0ffffffff  2038-01-19..2106-02-07
- * 0 1    0    0x100000000..0x17fffffff  2106-02-07..2174-02-25
- * 0 1    1    0x180000000..0x1ffffffff  2174-02-25..2242-03-16
- * 1 0    0    0x200000000..0x27fffffff  2242-03-16..2310-04-04
- * 1 0    1    0x280000000..0x2ffffffff  2310-04-04..2378-04-22
- * 1 1    0    0x300000000..0x37fffffff  2378-04-22..2446-05-10
-
- * 1 1    1    -0x80000000..-0x00000001  1901-12-13..1969-12-31
+ * extra  msb of                         adjust for signed
+ * epoch  32-bit                         32-bit tv_sec to
+ * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
+ * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
+ * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
+ * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
+ * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
+ * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
+ * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
+ * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
+ * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
+ *
+ * Note that previous versions of the kernel on 64-bit systems would
+ * incorrectly use extra epoch bits 1,1 for dates between 1901 and
+ * 1970.  e2fsck will correct this, assuming that it is run on the
+ * affected filesystem before 2311.
  */
 
 static inline __le32 ext4_encode_extra_time(struct timespec *time)
 {
-       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
-			   (time->tv_sec >> 32) & EXT4_EPOCH_MASK : 0) |
-                          ((time->tv_nsec << EXT4_EPOCH_BITS) & EXT4_NSEC_MASK));
+	u32 extra = sizeof(time->tv_sec) > 4 ?
+		((time->tv_sec - (s32)time->tv_sec) >> 32) & EXT4_EPOCH_MASK : 0;
+	return cpu_to_le32(extra | (time->tv_nsec << EXT4_EPOCH_BITS));
 }
 
 static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
 {
-       if (sizeof(time->tv_sec) > 4)
-	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
-			       << 32;
-       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
+	if (unlikely(sizeof(time->tv_sec) > 4 &&
+			(extra & cpu_to_le32(EXT4_EPOCH_MASK)))) {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,20,0)
+		/* Handle legacy encoding of pre-1970 dates with epoch
+		 * bits 1,1.  We assume that by kernel version 4.20,
+		 * everyone will have run fsck over the affected
+		 * filesystems to correct the problem.
+		 */
+		u64 extra_bits = le32_to_cpu(extra) & EXT4_EPOCH_MASK;
+		if (extra_bits == 3)
+			extra_bits = 0;
+		time->tv_sec += extra_bits << 32;
+#else
+		time->tv_sec += (u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;
+#endif
+	}
+	time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
 }
 
 #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
-- 
1.8.1.2

[PATCH v4 2/2] e2fsck: Correct ext4 dates generated by old kernels.

[David Turner]

This patch is against e2fsprogs.

---
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 2311).

Signed-off-by: David Turner <novalis@novalis.org>
---
 e2fsck/pass1.c   | 37 +++++++++++++++++++++++++++++++++++++
 e2fsck/problem.c |  7 +++++++
 e2fsck/problem.h |  6 ++++++
 3 files changed, 50 insertions(+)

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index ab23e42..cb72964 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -348,6 +348,23 @@ fix:
 				EXT2_INODE_SIZE(sb), "pass1");
 }
 
+#define EXT4_EPOCH_BITS 2
+#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
+
+static int large_inode_extra(__u32 xtime, __u32 extra) {
+	return (xtime & (1 << 31)) != 0 &&
+		(extra & EXT4_EPOCH_MASK) == EXT4_EPOCH_MASK;
+}
+
+#define LARGE_INODE_EXTRA(inode, xtime) \
+	large_inode_extra(inode->i_##xtime, \
+			  inode->i_##xtime##_extra)
+
+/* When the date is earlier than 2311, we assume that atimes, ctimes,
+ * and mtimes greater than 2311 are actually pre-1970 dates mis-encoded.
+ */
+#define EXT4_EXTRA_NEGATIVE_DATE_CUTOFF 6 * (1UL << 32)
+
 static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx)
 {
 	struct ext2_super_block *sb = ctx->fs->super;
@@ -388,6 +405,26 @@ 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, the inode is corrupt.
+	 */
+	if (sizeof(time_t) > 4 && ctx->now < EXT4_EXTRA_NEGATIVE_DATE_CUTOFF &&
+		LARGE_INODE_EXTRA(inode, atime) ||
+		LARGE_INODE_EXTRA(inode, ctime) ||
+		LARGE_INODE_EXTRA(inode, mtime)) {
+
+		if (!fix_problem(ctx, PR_1_EA_TIME_OUT_OF_RANGE, pctx))
+			return;
+
+		inode->i_atime_extra &= ~EXT4_EPOCH_MASK;
+		inode->i_ctime_extra &= ~EXT4_EPOCH_MASK;
+		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..51fa7c3 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -1018,6 +1018,13 @@ 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 },
 
+/* The extended a, c, or mtime on this inode is in the far future,
+   indicating that it was written with an older, buggy version of the
+   kernel on a 64-bit machine */
+	{ PR_1_EA_TIME_OUT_OF_RANGE,
+	  N_("Extended time on @i %i is in the far future.\n"
+		"Assume that it is in fact a pre-1970 date written by an older, buggy version of Linux?\n"), 
+		PROMPT_FIX, 0 },
 
 	/* Pass 1b errors */
 
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index ae1ed26..a44f6dd 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -593,6 +593,12 @@ struct problem_context {
 #define PR_1_EXTENT_INDEX_START_INVALID	0x01006D
 
 #define PR_1_EXTENT_END_OUT_OF_BOUNDS	0x01006E
+
+/* The extended a, c, or mtime on this inode is in the far future,
+   indicating that it was written with an older, buggy version of
+	 the kernel on a 64-bit machine */
+#define PR_1_EA_TIME_OUT_OF_RANGE	0x01006F
+
 /*
  * Pass 1b errors
  */
-- 
1.8.1.2

Re: [PATCH v4 2/2] e2fsck: Correct ext4 dates generated by old kernels.

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 5520 bytes --]

On Nov 13, 2013, at 12:00 AM, David Turner <novalis@novalis.org> wrote:
> This patch is against e2fsprogs.
> 
> ---
> 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 2311).
> 
> Signed-off-by: David Turner <novalis@novalis.org>
> ---
> e2fsck/pass1.c   | 37 +++++++++++++++++++++++++++++++++++++
> e2fsck/problem.c |  7 +++++++
> e2fsck/problem.h |  6 ++++++
> 3 files changed, 50 insertions(+)
> 
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index ab23e42..cb72964 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -348,6 +348,23 @@ fix:
> 				EXT2_INODE_SIZE(sb), "pass1");
> }
> 
> +#define EXT4_EPOCH_BITS 2
> +#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
> +
> +static int large_inode_extra(__u32 xtime, __u32 extra) {

“large_inode_extra()” doesn’t really describe the purpose of this function very
well, which is checking the timestamps to have large future (or past) dates.
How about a more descriptive name “check_old_ext4_negative_epoch()” or
maybe “check_inode_extra_negative_epoch()” or something like that?

> +	return (xtime & (1 << 31)) != 0 &&
> +		(extra & EXT4_EPOCH_MASK) == EXT4_EPOCH_MASK;
> +}
> +
> +#define LARGE_INODE_EXTRA(inode, xtime) \

Ditto.

> +	large_inode_extra(inode->i_##xtime, \
> +			  inode->i_##xtime##_extra)
> +
> +/* When the date is earlier than 2311, we assume that atimes, ctimes,
> + * and mtimes greater than 2311 are actually pre-1970 dates mis-encoded.

I like the idea of checking the current date, so that there isn’t a need to
revert this code at some point in the future.  I’m wondering if there should
be a margin, like “When the current date is earlier than 2240 we assume ...
times greater than 2311 are actually ...”?   I’d hope that old versions of
pre-3.14 kernels are not still running by then.

> +#define EXT4_EXTRA_NEGATIVE_DATE_CUTOFF 6 * (1UL << 32)

That would make this (5 * (1ULL << 32)).  I think this should be ULL so that
if there are 64-bit timestamps on 32-bit systems it will still work correctly.

> static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx)
> {
> 	struct ext2_super_block *sb = ctx->fs->super;
> @@ -388,6 +405,26 @@ 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, the inode is corrupt.
> +	 */
> +	if (sizeof(time_t) > 4 && ctx->now < EXT4_EXTRA_NEGATIVE_DATE_CUTOFF &&
> +		LARGE_INODE_EXTRA(inode, atime) ||
> +		LARGE_INODE_EXTRA(inode, ctime) ||
> +		LARGE_INODE_EXTRA(inode, mtime)) {

(style) please align continued line after ‘(‘ of previous line, otherwise it isn’t
easy to see if these are continuations of the condition or if they are part of the
body of the condition like the lines below.

> +		if (!fix_problem(ctx, PR_1_EA_TIME_OUT_OF_RANGE, pctx))
> +			return;
> +
> +		inode->i_atime_extra &= ~EXT4_EPOCH_MASK;
> +		inode->i_ctime_extra &= ~EXT4_EPOCH_MASK;
> +		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..51fa7c3 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -1018,6 +1018,13 @@ 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 },
> 
> +/* The extended a, c, or mtime on this inode is in the far future,
> +   indicating that it was written with an older, buggy version of the
> +   kernel on a 64-bit machine */

Please make the comment match the expanded text as closely as possible.
Otherwise, it is hard to track down some problem that prints one message,
but uses the crazy @foo encodings and it isn’t clear what part of e2fsck
generated it.  It is fine if it contains more text.

> +	{ PR_1_EA_TIME_OUT_OF_RANGE,
> +	  N_("Extended time on @i %i is in the far future.\n"
> +		"Assume that it is in fact a pre-1970 date written by an older, buggy version of Linux?\n"), 

(style) please align after ‘(‘ from previous line and under 80 columns.

That said, I think this message is a bit harsh, since those older, buggy versions
of Linux include all versions running today.  I’d probably make a more succinct
message like:

           N_(“Timestamp(s) on @i %i beyond 2033 are likely pre-1970 dates.\n”)

> +		PROMPT_FIX, 0 },

I’d probably also make this error code “PROMPT_FIX | PREEN_OK | PR_NO_OK”.

> diff --git a/e2fsck/problem.h b/e2fsck/problem.h
> index ae1ed26..a44f6dd 100644
> --- a/e2fsck/problem.h
> +++ b/e2fsck/problem.h
> @@ -593,6 +593,12 @@ struct problem_context {
> #define PR_1_EXTENT_INDEX_START_INVALID	0x01006D
> 
> #define PR_1_EXTENT_END_OUT_OF_BOUNDS	0x01006E
> +
> +/* The extended a, c, or mtime on this inode is in the far future,
> +   indicating that it was written with an older, buggy version of
> +	 the kernel on a 64-bit machine */
> +#define PR_1_EA_TIME_OUT_OF_RANGE	0x01006F

Same goes for the comment here - it should contain the exact text of the printed
error message.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH v4 1/2] ext4: Fix handling of extended tv_sec (bug 23732)

[Darrick J. Wong]

On Wed, Nov 13, 2013 at 02:00:15AM -0500, David Turner wrote:
> In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
> the {a,c,m}time fields, deferring the year 2038 problem to the year
> 2446.
> 
> When decoding these extended fields, for times whose bottom 32 bits
> would represent a negative number, sign extension causes the 64-bit
> extended timestamp to be negative as well, which is not what's
> intended.  This patch corrects that issue, so that the only negative
> {a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
> timestamps).
> 
> Some older kernels might have written pre-1970 dates with 1,1 in the
> extra bits.  This patch treats those incorrectly-encoded dates as
> pre-1970, instead of post-2311, until kernel 4.20 is released.
> Hopefully by then e2fsck will have fixed up the bad data.
> 
> Signed-off-by: David Turner <novalis@novalis.org>
> Reported-by: Mark Harris <mh8928@yahoo.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
> ---
>  fs/ext4/ext4.h | 61 +++++++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 39 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 18aa56b..7d5e019 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -26,6 +26,7 @@
>  #include <linux/seqlock.h>
>  #include <linux/mutex.h>
>  #include <linux/timer.h>
> +#include <linux/version.h>
>  #include <linux/wait.h>
>  #include <linux/blockgroup_lock.h>
>  #include <linux/percpu_counter.h>
> @@ -713,38 +714,54 @@ struct move_extent {
>  	  sizeof((ext4_inode)->field))			\
>  	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
>  	    (einode)->i_extra_isize))			\
> +
>  /*
> - * We use the bottom 34 bits of the signed 64-bit time value, with
> - * the top two of these bits in the bottom of extra.  This leads
> - * to a slightly odd encoding, which works like this:
> + * We need is an encoding that preserves the times for extra epoch "00":
>   *
> - * extra  msb of
> - * epoch  32-bit
> - * bits   time    decoded 64-bit tv_sec   valid time range
> - * 0 0    0    0x000000000..0x07fffffff  1970-01-01..2038-01-19
> - * 0 0    1    0x080000000..0x0ffffffff  2038-01-19..2106-02-07
> - * 0 1    0    0x100000000..0x17fffffff  2106-02-07..2174-02-25
> - * 0 1    1    0x180000000..0x1ffffffff  2174-02-25..2242-03-16
> - * 1 0    0    0x200000000..0x27fffffff  2242-03-16..2310-04-04
> - * 1 0    1    0x280000000..0x2ffffffff  2310-04-04..2378-04-22
> - * 1 1    0    0x300000000..0x37fffffff  2378-04-22..2446-05-10
> -
> - * 1 1    1    -0x80000000..-0x00000001  1901-12-13..1969-12-31
> + * extra  msb of                         adjust for signed
> + * epoch  32-bit                         32-bit tv_sec to
> + * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
> + * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
> + * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
> + * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
> + * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
> + * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
> + * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
> + * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
> + * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
> + *
> + * Note that previous versions of the kernel on 64-bit systems would
> + * incorrectly use extra epoch bits 1,1 for dates between 1901 and
> + * 1970.  e2fsck will correct this, assuming that it is run on the
> + * affected filesystem before 2311.

Given the table, should this be "before 2310-04-04"?

--D
>   */
>  
>  static inline __le32 ext4_encode_extra_time(struct timespec *time)
>  {
> -       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
> -			   (time->tv_sec >> 32) & EXT4_EPOCH_MASK : 0) |
> -                          ((time->tv_nsec << EXT4_EPOCH_BITS) & EXT4_NSEC_MASK));
> +	u32 extra = sizeof(time->tv_sec) > 4 ?
> +		((time->tv_sec - (s32)time->tv_sec) >> 32) & EXT4_EPOCH_MASK : 0;
> +	return cpu_to_le32(extra | (time->tv_nsec << EXT4_EPOCH_BITS));
>  }
>  
>  static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
>  {
> -       if (sizeof(time->tv_sec) > 4)
> -	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> -			       << 32;
> -       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
> +	if (unlikely(sizeof(time->tv_sec) > 4 &&
> +			(extra & cpu_to_le32(EXT4_EPOCH_MASK)))) {
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,20,0)
> +		/* Handle legacy encoding of pre-1970 dates with epoch
> +		 * bits 1,1.  We assume that by kernel version 4.20,
> +		 * everyone will have run fsck over the affected
> +		 * filesystems to correct the problem.
> +		 */
> +		u64 extra_bits = le32_to_cpu(extra) & EXT4_EPOCH_MASK;
> +		if (extra_bits == 3)
> +			extra_bits = 0;
> +		time->tv_sec += extra_bits << 32;
> +#else
> +		time->tv_sec += (u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;
> +#endif
> +	}
> +	time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
>  }
>  
>  #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
> -- 
> 1.8.1.2
> 
> 
> 
> --
> 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 v5 1/2] ext4: Fix handling of extended tv_sec (bug 23732)

[David Turner]

Thanks for your patient and detailed comments.
--

In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
the {a,c,m}time fields, deferring the year 2038 problem to the year
2446.

When decoding these extended fields, for times whose bottom 32 bits
would represent a negative number, sign extension causes the 64-bit
extended timestamp to be negative as well, which is not what's
intended.  This patch corrects that issue, so that the only negative
{a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
timestamps).

Some older kernels might have written pre-1970 dates with 1,1 in the
extra bits.  This patch treats those incorrectly-encoded dates as
pre-1970, instead of post-2311, until kernel 4.20 is released.
Hopefully by then e2fsck will have fixed up the bad data.

Signed-off-by: David Turner <novalis@novalis.org>
Reported-by: Mark Harris <mh8928@yahoo.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
---
 fs/ext4/ext4.h | 61 +++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 18aa56b..ac54dac 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -26,6 +26,7 @@
 #include <linux/seqlock.h>
 #include <linux/mutex.h>
 #include <linux/timer.h>
+#include <linux/version.h>
 #include <linux/wait.h>
 #include <linux/blockgroup_lock.h>
 #include <linux/percpu_counter.h>
@@ -713,38 +714,54 @@ struct move_extent {
 	  sizeof((ext4_inode)->field))			\
 	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
 	    (einode)->i_extra_isize))			\
+
 /*
- * We use the bottom 34 bits of the signed 64-bit time value, with
- * the top two of these bits in the bottom of extra.  This leads
- * to a slightly odd encoding, which works like this:
+ * We need is an encoding that preserves the times for extra epoch "00":
  *
- * extra  msb of
- * epoch  32-bit
- * bits   time    decoded 64-bit tv_sec   valid time range
- * 0 0    0    0x000000000..0x07fffffff  1970-01-01..2038-01-19
- * 0 0    1    0x080000000..0x0ffffffff  2038-01-19..2106-02-07
- * 0 1    0    0x100000000..0x17fffffff  2106-02-07..2174-02-25
- * 0 1    1    0x180000000..0x1ffffffff  2174-02-25..2242-03-16
- * 1 0    0    0x200000000..0x27fffffff  2242-03-16..2310-04-04
- * 1 0    1    0x280000000..0x2ffffffff  2310-04-04..2378-04-22
- * 1 1    0    0x300000000..0x37fffffff  2378-04-22..2446-05-10
-
- * 1 1    1    -0x80000000..-0x00000001  1901-12-13..1969-12-31
+ * extra  msb of                         adjust for signed
+ * epoch  32-bit                         32-bit tv_sec to
+ * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
+ * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
+ * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
+ * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
+ * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
+ * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
+ * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
+ * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
+ * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
+ *
+ * Note that previous versions of the kernel on 64-bit systems would
+ * incorrectly use extra epoch bits 1,1 for dates between 1901 and
+ * 1970.  e2fsck will correct this, assuming that it is run on the
+ * affected filesystem before 2242.
  */
 
 static inline __le32 ext4_encode_extra_time(struct timespec *time)
 {
-       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
-			   (time->tv_sec >> 32) & EXT4_EPOCH_MASK : 0) |
-                          ((time->tv_nsec << EXT4_EPOCH_BITS) & EXT4_NSEC_MASK));
+	u32 extra = sizeof(time->tv_sec) > 4 ?
+		((time->tv_sec - (s32)time->tv_sec) >> 32) & EXT4_EPOCH_MASK : 0;
+	return cpu_to_le32(extra | (time->tv_nsec << EXT4_EPOCH_BITS));
 }
 
 static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
 {
-       if (sizeof(time->tv_sec) > 4)
-	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
-			       << 32;
-       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
+	if (unlikely(sizeof(time->tv_sec) > 4 &&
+			(extra & cpu_to_le32(EXT4_EPOCH_MASK)))) {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,20,0)
+		/* Handle legacy encoding of pre-1970 dates with epoch
+		 * bits 1,1.  We assume that by kernel version 4.20,
+		 * everyone will have run fsck over the affected
+		 * filesystems to correct the problem.
+		 */
+		u64 extra_bits = le32_to_cpu(extra) & EXT4_EPOCH_MASK;
+		if (extra_bits == 3)
+			extra_bits = 0;
+		time->tv_sec += extra_bits << 32;
+#else
+		time->tv_sec += (u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;
+#endif
+	}
+	time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
 }
 
 #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
-- 
1.8.1.2

[PATCH v5 2/2] e2fsck: Correct ext4 dates generated by old kernels.

[David Turner]

(apologies if this is a dup -- my mail client crashed and I don't see
this one in the lkml archives)
--
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@novalis.org>
---
 e2fsck/pass1.c   | 38 ++++++++++++++++++++++++++++++++++++++
 e2fsck/problem.c |  4 ++++
 e2fsck/problem.h |  4 ++++
 3 files changed, 46 insertions(+)

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index ab23e42..b5ae885 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 greater than 2242 are actually pre-1970 dates
+ * mis-encoded.
+ */
+#define EXT4_EXTRA_NEGATIVE_DATE_CUTOFF 5 * (1ULL << 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,26 @@ 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, the inode is corrupt.
+	 */
+	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;
+
+		inode->i_atime_extra &= ~EXT4_EPOCH_MASK;
+		inode->i_ctime_extra &= ~EXT4_EPOCH_MASK;
+		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
  */
-- 
1.8.1.2

Re: [PATCH v5 2/2] e2fsck: Correct ext4 dates generated by old kernels.

[Mark Harris]

> +/* When today's date is earlier than 2242, we assume that atimes,
> + * ctimes, and mtimes greater than 2242 are actually pre-1970 dates

...mtimes with years in the range 2310..2378 are...

> + * mis-encoded.
> + */
> +#define EXT4_EXTRA_NEGATIVE_DATE_CUTOFF 5 * (1ULL << 32)

Wouldn't 2242 be 0x200000000ULL, i.e. 2 * (1ULL << 32)?


> +
> +       /*
> +        * If the inode's extended atime (ctime, mtime) is stored in
> +        * the old, invalid format, the inode is corrupt.
> +        */
> +       if (sizeof(time_t) > 4 && ctx->now < EXT4_EXTRA_NEGATIVE_DATE_CUTOFF &&

Needs parentheses (

> +           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;
> +
> +               inode->i_atime_extra &= ~EXT4_EPOCH_MASK;
> +               inode->i_ctime_extra &= ~EXT4_EPOCH_MASK;
> +               inode->i_mtime_extra &= ~EXT4_EPOCH_MASK;

With e.g. 2039 atime and 1968 mtime, shouldn't only i_mtime_extra be altered?

> +               e2fsck_write_inode_full(ctx, pctx->ino, pctx->inode,
> +                                       EXT2_INODE_SIZE(sb), "pass1");
> +       }

 - Mark

[PATCH v6] e2fsck: Correct ext4 dates generated by old kernels.

[David Turner]

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@novalis.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
  */
-- 
1.8.1.2

Re: [PATCH v6] e2fsck: Correct ext4 dates generated by old kernels.

[David Turner]

Is this version good, or should I make some more improvements?

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@novalis.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
>   */

Re: [PATCH v6] e2fsck: Correct ext4 dates generated by old kernels.

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 5644 bytes --]

On Nov 29, 2013, at 2:54 PM, David Turner <novalis@novalis.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@novalis.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






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v7 1/2] e2fsck: Correct ext4 dates generated by old kernels.

[David Turner]

I went ahead and wrote some tests, and they seem to confirm that
my patch to e2fsck works as expected (once I added crtime).

However, as Andreas notes, "we want to verify .. that "debugfs -R
'stat testfile'" decodes the times correctly."  Unfortunately, it
does not, and it is not trivial to fix.  debugfs uses an internal
function time_to_string(__u32), which calls gmtime or localtime.
These functions are defined on time_t, which is 32-bits on 32-bit
Linux systems.  In addition, because time_to_string takes
an unsigned int, it returns bad results for pre-1970 dates.

One fix for this would be to include our own gmtime/localtime
defined on 64-bit integers.  But this is likely to be
complicated, given the dependence of localtime on timezone data.

Instead, I decided to simply adjust time_to_string to take a
time_t.  This means that on 32-bit time_t systems, we will give
wrong results for post-2038 dates, but correct results for
pre-1970 dates. (see patch 2/2)

For the tests, I am looking at the raw value of ctime and
ctime_extra, which will work regardless of the size of time_t

Is using time_t acceptable, or do you want me to copy over
gmtime/localtime from glibc?  Or is there a third approach
that would be better?

Also, while I was making this change, I happened to notice that
there is no dtime_extra field into which to put the extra bits.
This means that there is still a year 2038 problem with tools
that deal with deleted files (including the corner case that the
system is shut down cleanly at the exact second that the bottom
32 bits of the current time are zero, leading fsck to believe
that there was an unclean shutdown).

I want to make an additional change to correct this.  Since I
assume it is impossible to add an additional field, I propose to
use ctime_extra to store the extra bits of dtime on deletion.
This is a bit hackish, since it does destroy a bit of data, but
it is significantly better than dtime being totally broken after
2038.  What do people think?
--
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).

Includes tests for this, as well as changes to debugfs to correctly
set crtimes.

Signed-off-by: David Turner <novalis@novalis.org>
---
 debugfs/set_fields.c                  |  2 +-
 e2fsck/pass1.c                        | 43 ++++++++++++++++
 e2fsck/problem.c                      |  4 ++
 e2fsck/problem.h                      |  4 ++
 lib/extra_epoch.h                     |  2 +
 tests/f_pre_1970_date_encoding/expect | 45 ++++++++++++++++
 tests/f_pre_1970_date_encoding/name   |  1 +
 tests/f_pre_1970_date_encoding/script | 96 +++++++++++++++++++++++++++++++++++
 8 files changed, 196 insertions(+), 1 deletion(-)
 create mode 100644 lib/extra_epoch.h
 create mode 100644 tests/f_pre_1970_date_encoding/expect
 create mode 100644 tests/f_pre_1970_date_encoding/name
 create mode 100644 tests/f_pre_1970_date_encoding/script

diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
index aad1cd8..f7c55a7 100644
--- a/debugfs/set_fields.c
+++ b/debugfs/set_fields.c
@@ -200,7 +200,7 @@ static struct field_set_info inode_fields[] = {
 		4, parse_uint },
 	{ "atime_extra", &set_inode.i_atime_extra, NULL,
 		4, parse_uint },
-	{ "crtime", &set_inode.i_crtime, NULL, 4, parse_uint },
+	{ "crtime", &set_inode.i_crtime, NULL, 4, parse_time },
 	{ "crtime_extra", &set_inode.i_crtime_extra, NULL,
 		4, parse_uint },
 	{ "bmap", NULL, NULL, 4, parse_bmap, FLAG_ARRAY },
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index ab23e42..ecbd79e 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -50,6 +50,8 @@
 
 #include "problem.h"
 
+#include "extra_epoch.h"
+
 #ifdef NO_INLINE_FUNCS
 #define _INLINE_
 #else
@@ -348,6 +350,21 @@ fix:
 				EXT2_INODE_SIZE(sb), "pass1");
 }
 
+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, crtimes, 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 +405,32 @@ 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, crtime, 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, crtime) ||
+	     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, crtime))
+			inode->i_crtime_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
  */
diff --git a/lib/extra_epoch.h b/lib/extra_epoch.h
new file mode 100644
index 0000000..465c43f
--- /dev/null
+++ b/lib/extra_epoch.h
@@ -0,0 +1,2 @@
+#define EXT4_EPOCH_BITS 2
+#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
diff --git a/tests/f_pre_1970_date_encoding/expect b/tests/f_pre_1970_date_encoding/expect
new file mode 100644
index 0000000..1a71571
--- /dev/null
+++ b/tests/f_pre_1970_date_encoding/expect
@@ -0,0 +1,45 @@
+times for year-1909 =
+ ctime: 0x8e475440:00000003
+ atime: 0x8e475440:00000003
+ mtime: 0x8e475440:00000003
+crtime: 0x8e475440:00000003
+times for year-1979 =
+ ctime: 0x11db6940:00000000
+ atime: 0x11db6940:00000000
+ mtime: 0x11db6940:00000000
+crtime: 0x11db6940:00000000
+times for year-2039 =
+ ctime: 0x82a37b40:00000001
+ atime: 0x82a37b40:00000001
+ mtime: 0x82a37b40:00000001
+crtime: 0x82a37b40:00000001
+times for year-2139 =
+ ctime: 0x3e9b9940:00000001
+ atime: 0x3e9b9940:00000001
+ mtime: 0x3e9b9940:00000001
+crtime: 0x3e9b9940:00000001
+times for year-1909 =
+ ctime: 0x8e475440:00000000
+ atime: 0x8e475440:00000000
+ mtime: 0x8e475440:00000000
+crtime: 0x8e475440:00000000
+times for year-1979 =
+ ctime: 0x11db6940:00000000
+ atime: 0x11db6940:00000000
+ mtime: 0x11db6940:00000000
+crtime: 0x11db6940:00000000
+times for year-2039 =
+ ctime: 0x82a37b40:00000001
+ atime: 0x82a37b40:00000001
+ mtime: 0x82a37b40:00000001
+crtime: 0x82a37b40:00000001
+times for year-2139 =
+ ctime: 0x3e9b9940:00000001
+ atime: 0x3e9b9940:00000001
+ mtime: 0x3e9b9940:00000001
+crtime: 0x3e9b9940:00000001
+times for year-1909 =
+ ctime: 0x8e475440:00000003
+ atime: 0x8e475440:00000003
+ mtime: 0x8e475440:00000003
+crtime: 0x8e475440:00000003
diff --git a/tests/f_pre_1970_date_encoding/name b/tests/f_pre_1970_date_encoding/name
new file mode 100644
index 0000000..9805324
--- /dev/null
+++ b/tests/f_pre_1970_date_encoding/name
@@ -0,0 +1 @@
+correct mis-encoded pre-1970 dates
diff --git a/tests/f_pre_1970_date_encoding/script b/tests/f_pre_1970_date_encoding/script
new file mode 100644
index 0000000..c3e12f5
--- /dev/null
+++ b/tests/f_pre_1970_date_encoding/script
@@ -0,0 +1,96 @@
+if test -x $DEBUGFS_EXE; then
+
+OUT=$test_name.log
+TIMESTAMPS=$test_name.timestamps.log
+EXP=$test_dir/expect
+FSCK_OPT=-yf
+
+create_file_with_xtime_and_extra() {
+    name=$1
+    time=$2
+    extra=$3
+    $DEBUGFS -w -R "write /dev/null $name" $TMPFILE > $OUT 2>&1
+    for xtime in atime ctime mtime crtime
+    do
+        $DEBUGFS -w -R "set_inode_field $name $xtime @$time" $TMPFILE > $OUT 2>&1
+
+        $DEBUGFS -w -R "set_inode_field $name ${xtime}_extra $extra" $TMPFILE > $OUT 2>&1
+    done
+}
+
+get_file_xtime_and_extra() {
+    name=$1
+    echo "times for $name =" >> $TIMESTAMPS
+    $DEBUGFS -R "stat $name" $TMPFILE 2>&1 | egrep '^( a| c| m|cr)time: ' |sed 's/ --.*//' >> $TIMESTAMPS
+}
+
+rm -f $OUT
+rm -f $TIMESTAMPS
+
+#create an empty ext4 filesystem with 256-byte inodes for testing
+dd status=none if=/dev/zero of=$TMPFILE bs=1024 count=5000
+echo  mkfs.ext4 -q -I 256 $TMPFILE >> $OUT
+yes | mkfs.ext4 -q -I 256 $TMPFILE >> $OUT 2>&1
+
+# this is a pre-1970 file encoded with the old encoding.
+# fsck should repair this
+create_file_with_xtime_and_extra year-1909 -1907928000 3
+
+# these are all already encoded correctly
+create_file_with_xtime_and_extra year-1979   299592000 0
+create_file_with_xtime_and_extra year-2039  2191752000 1
+create_file_with_xtime_and_extra year-2139  5345352000 1
+
+# confirm that the xtime is wrong on the pre-1970 file
+get_file_xtime_and_extra year-1909
+
+# and confirm that it is right on the remaining files
+get_file_xtime_and_extra year-1979
+get_file_xtime_and_extra year-2039
+get_file_xtime_and_extra year-2139
+
+# before we repair the filesystem, save off a copy so that
+# we can use it later
+
+cp $TMPFILE $TMPFILE.backup
+
+# repair the filesystem
+E2FSCK_TIME=1386393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
+
+# check that the dates and xtime_extra on the file is now correct
+get_file_xtime_and_extra year-1909
+
+# check that the remaining dates have not been altered
+get_file_xtime_and_extra year-1979
+get_file_xtime_and_extra year-2039
+get_file_xtime_and_extra year-2139
+
+# now we need to check that after the year 2242, e2fsck does not
+# modify dates with extra_xtime=3
+
+# restore the unrepaired filesystem
+mv $TMPFILE.backup $TMPFILE
+
+#retry the repair
+E2FSCK_TIME=9270393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
+
+# check that the 1909 file is unaltered (i.e. it has a post-2378 date)
+get_file_xtime_and_extra year-1909
+
+cmp -s $TIMESTAMPS $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $TIMESTAMPS > $test_name.failed
+fi
+
+unset OUT TIMESTAMPS EXP FSCK_OPT
+
+else #if test -x $DEBUGFS_EXE; then
+	echo "$test_name: $test_description: skipped"
+fi
+
-- 
1.8.1.2

[PATCH v7 2/2] debugfs: Decode {a,c,cr,m}time_extra fields in stat

[David Turner]

Decodes post-2038 dates correctly on on machines with a 64-bit time_t.
When decoding dates from xtime+xtime_extra fields, we assume that
these dates are in the correct format (i.e. pre-1970 dates have
xtime_extra low bits == 0 instead of 3).  So uncorrected pre-1970
dates will be displayed as post-2310 dates.  This is because we don't
want our filesystem debugging tools to silently correct data errors.

Signed-off-by: David Turner <novalis@novalis.org>
---
 debugfs/debugfs.c | 22 ++++++++++++++++++----
 debugfs/debugfs.h |  2 +-
 debugfs/util.c    |  7 +++----
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index 8c32eff..aa2f972 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -37,6 +37,8 @@ extern char *optarg;
 #include "../version.h"
 #include "jfs_user.h"
 
+#include "extra_epoch.h"
+
 #ifndef BUFSIZ
 #define BUFSIZ 8192
 #endif
@@ -718,6 +720,14 @@ static void dump_extents(FILE *f, const char
*prefix, ext2_ino_t ino,
 		fprintf(f, "\n");
 }
 
+char* inode_time_to_string(__s32 xtime, __u32 xtime_extra) {
+	time_t time = (time_t) xtime;
+	if (sizeof(time_t) > 4) {
+		time += (xtime_extra & EXT4_EPOCH_MASK) << 32;
+	}
+	return time_to_string(time);
+}
+
 void internal_dump_inode(FILE *out, const char *prefix,
 			 ext2_ino_t inode_num, struct ext2_inode *inode,
 			 int do_dump_blocks)
@@ -791,16 +801,20 @@ void internal_dump_inode(FILE *out, const char
*prefix,
 	if (is_large_inode && large_inode->i_extra_isize >= 24) {
 		fprintf(out, "%s ctime: 0x%08x:%08x -- %s", prefix,
 			inode->i_ctime, large_inode->i_ctime_extra,
-			time_to_string(inode->i_ctime));
+			inode_time_to_string(inode->i_ctime,
+					     large_inode->i_ctime_extra));
 		fprintf(out, "%s atime: 0x%08x:%08x -- %s", prefix,
 			inode->i_atime, large_inode->i_atime_extra,
-			time_to_string(inode->i_atime));
+			inode_time_to_string(inode->i_atime,
+					     large_inode->i_atime_extra));
 		fprintf(out, "%s mtime: 0x%08x:%08x -- %s", prefix,
 			inode->i_mtime, large_inode->i_mtime_extra,
-			time_to_string(inode->i_mtime));
+			inode_time_to_string(inode->i_mtime,
+					     large_inode->i_mtime_extra));
 		fprintf(out, "%scrtime: 0x%08x:%08x -- %s", prefix,
 			large_inode->i_crtime, large_inode->i_crtime_extra,
-			time_to_string(large_inode->i_crtime));
+			inode_time_to_string(large_inode->i_crtime,
+					     large_inode->i_crtime_extra));
 	} else {
 		fprintf(out, "%sctime: 0x%08x -- %s", prefix, inode->i_ctime,
 			time_to_string(inode->i_ctime));
diff --git a/debugfs/debugfs.h b/debugfs/debugfs.h
index 45175cf..a02f29c 100644
--- a/debugfs/debugfs.h
+++ b/debugfs/debugfs.h
@@ -33,7 +33,7 @@ extern int check_fs_not_open(char *name);
 extern int check_fs_read_write(char *name);
 extern int check_fs_bitmaps(char *name);
 extern ext2_ino_t string_to_inode(char *str);
-extern char *time_to_string(__u32);
+extern char *time_to_string(time_t);
 extern time_t string_to_time(const char *);
 extern unsigned long parse_ulong(const char *str, const char *cmd,
 				 const char *descr, int *err);
diff --git a/debugfs/util.c b/debugfs/util.c
index cf3a6c6..a60aec7 100644
--- a/debugfs/util.c
+++ b/debugfs/util.c
@@ -187,13 +187,12 @@ int check_fs_bitmaps(char *name)
 }
 
 /*
- * This function takes a __u32 time value and converts it to a string,
- * using ctime
+ * This function takes a time_t time value and converts it to a string,
+ * using asctime
  */
-char *time_to_string(__u32 cl)
+char *time_to_string(time_t t)
 {
 	static int	do_gmt = -1;
-	time_t		t = (time_t) cl;
 	const char	*tz;
 
 	if (do_gmt == -1) {
-- 
1.8.1.2

Re: [PATCH v7 1/2] e2fsck: Correct ext4 dates generated by old kernels.

[Andreas Dilger]

I suspect that any 32-bit systems running at that time will have been updated to have 64-bit time_t or otherwise have windowed the 32-bit time_t to have a new starting epoch.

So I'm willing to punt on decoding the 64-bit value correctly to libc and just assign our time to the system time_t. 

Cheers, Andreas

> On Dec 7, 2013, at 13:02, David Turner <novalis@novalis.org> wrote:
> 
> I went ahead and wrote some tests, and they seem to confirm that
> my patch to e2fsck works as expected (once I added crtime).
> 
> However, as Andreas notes, "we want to verify .. that "debugfs -R
> 'stat testfile'" decodes the times correctly."  Unfortunately, it
> does not, and it is not trivial to fix.  debugfs uses an internal
> function time_to_string(__u32), which calls gmtime or localtime.
> These functions are defined on time_t, which is 32-bits on 32-bit
> Linux systems.  In addition, because time_to_string takes
> an unsigned int, it returns bad results for pre-1970 dates.
> 
> One fix for this would be to include our own gmtime/localtime
> defined on 64-bit integers.  But this is likely to be
> complicated, given the dependence of localtime on timezone data.
> 
> Instead, I decided to simply adjust time_to_string to take a
> time_t.  This means that on 32-bit time_t systems, we will give
> wrong results for post-2038 dates, but correct results for
> pre-1970 dates. (see patch 2/2)
> 
> For the tests, I am looking at the raw value of ctime and
> ctime_extra, which will work regardless of the size of time_t
> 
> Is using time_t acceptable, or do you want me to copy over
> gmtime/localtime from glibc?  Or is there a third approach
> that would be better?
> 
> Also, while I was making this change, I happened to notice that
> there is no dtime_extra field into which to put the extra bits.
> This means that there is still a year 2038 problem with tools
> that deal with deleted files (including the corner case that the
> system is shut down cleanly at the exact second that the bottom
> 32 bits of the current time are zero, leading fsck to believe
> that there was an unclean shutdown).
> 
> I want to make an additional change to correct this.  Since I
> assume it is impossible to add an additional field, I propose to
> use ctime_extra to store the extra bits of dtime on deletion.
> This is a bit hackish, since it does destroy a bit of data, but
> it is significantly better than dtime being totally broken after
> 2038.  What do people think?
> --
> 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).
> 
> Includes tests for this, as well as changes to debugfs to correctly
> set crtimes.
> 
> Signed-off-by: David Turner <novalis@novalis.org>
> ---
> debugfs/set_fields.c                  |  2 +-
> e2fsck/pass1.c                        | 43 ++++++++++++++++
> e2fsck/problem.c                      |  4 ++
> e2fsck/problem.h                      |  4 ++
> lib/extra_epoch.h                     |  2 +
> tests/f_pre_1970_date_encoding/expect | 45 ++++++++++++++++
> tests/f_pre_1970_date_encoding/name   |  1 +
> tests/f_pre_1970_date_encoding/script | 96 +++++++++++++++++++++++++++++++++++
> 8 files changed, 196 insertions(+), 1 deletion(-)
> create mode 100644 lib/extra_epoch.h
> create mode 100644 tests/f_pre_1970_date_encoding/expect
> create mode 100644 tests/f_pre_1970_date_encoding/name
> create mode 100644 tests/f_pre_1970_date_encoding/script
> 
> diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
> index aad1cd8..f7c55a7 100644
> --- a/debugfs/set_fields.c
> +++ b/debugfs/set_fields.c
> @@ -200,7 +200,7 @@ static struct field_set_info inode_fields[] = {
>        4, parse_uint },
>    { "atime_extra", &set_inode.i_atime_extra, NULL,
>        4, parse_uint },
> -    { "crtime", &set_inode.i_crtime, NULL, 4, parse_uint },
> +    { "crtime", &set_inode.i_crtime, NULL, 4, parse_time },
>    { "crtime_extra", &set_inode.i_crtime_extra, NULL,
>        4, parse_uint },
>    { "bmap", NULL, NULL, 4, parse_bmap, FLAG_ARRAY },
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index ab23e42..ecbd79e 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -50,6 +50,8 @@
> 
> #include "problem.h"
> 
> +#include "extra_epoch.h"
> +
> #ifdef NO_INLINE_FUNCS
> #define _INLINE_
> #else
> @@ -348,6 +350,21 @@ fix:
>                EXT2_INODE_SIZE(sb), "pass1");
> }
> 
> +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, crtimes, 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 +405,32 @@ 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, crtime, 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, crtime) ||
> +         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, crtime))
> +            inode->i_crtime_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
>  */
> diff --git a/lib/extra_epoch.h b/lib/extra_epoch.h
> new file mode 100644
> index 0000000..465c43f
> --- /dev/null
> +++ b/lib/extra_epoch.h
> @@ -0,0 +1,2 @@
> +#define EXT4_EPOCH_BITS 2
> +#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
> diff --git a/tests/f_pre_1970_date_encoding/expect b/tests/f_pre_1970_date_encoding/expect
> new file mode 100644
> index 0000000..1a71571
> --- /dev/null
> +++ b/tests/f_pre_1970_date_encoding/expect
> @@ -0,0 +1,45 @@
> +times for year-1909 =
> + ctime: 0x8e475440:00000003
> + atime: 0x8e475440:00000003
> + mtime: 0x8e475440:00000003
> +crtime: 0x8e475440:00000003
> +times for year-1979 =
> + ctime: 0x11db6940:00000000
> + atime: 0x11db6940:00000000
> + mtime: 0x11db6940:00000000
> +crtime: 0x11db6940:00000000
> +times for year-2039 =
> + ctime: 0x82a37b40:00000001
> + atime: 0x82a37b40:00000001
> + mtime: 0x82a37b40:00000001
> +crtime: 0x82a37b40:00000001
> +times for year-2139 =
> + ctime: 0x3e9b9940:00000001
> + atime: 0x3e9b9940:00000001
> + mtime: 0x3e9b9940:00000001
> +crtime: 0x3e9b9940:00000001
> +times for year-1909 =
> + ctime: 0x8e475440:00000000
> + atime: 0x8e475440:00000000
> + mtime: 0x8e475440:00000000
> +crtime: 0x8e475440:00000000
> +times for year-1979 =
> + ctime: 0x11db6940:00000000
> + atime: 0x11db6940:00000000
> + mtime: 0x11db6940:00000000
> +crtime: 0x11db6940:00000000
> +times for year-2039 =
> + ctime: 0x82a37b40:00000001
> + atime: 0x82a37b40:00000001
> + mtime: 0x82a37b40:00000001
> +crtime: 0x82a37b40:00000001
> +times for year-2139 =
> + ctime: 0x3e9b9940:00000001
> + atime: 0x3e9b9940:00000001
> + mtime: 0x3e9b9940:00000001
> +crtime: 0x3e9b9940:00000001
> +times for year-1909 =
> + ctime: 0x8e475440:00000003
> + atime: 0x8e475440:00000003
> + mtime: 0x8e475440:00000003
> +crtime: 0x8e475440:00000003
> diff --git a/tests/f_pre_1970_date_encoding/name b/tests/f_pre_1970_date_encoding/name
> new file mode 100644
> index 0000000..9805324
> --- /dev/null
> +++ b/tests/f_pre_1970_date_encoding/name
> @@ -0,0 +1 @@
> +correct mis-encoded pre-1970 dates
> diff --git a/tests/f_pre_1970_date_encoding/script b/tests/f_pre_1970_date_encoding/script
> new file mode 100644
> index 0000000..c3e12f5
> --- /dev/null
> +++ b/tests/f_pre_1970_date_encoding/script
> @@ -0,0 +1,96 @@
> +if test -x $DEBUGFS_EXE; then
> +
> +OUT=$test_name.log
> +TIMESTAMPS=$test_name.timestamps.log
> +EXP=$test_dir/expect
> +FSCK_OPT=-yf
> +
> +create_file_with_xtime_and_extra() {
> +    name=$1
> +    time=$2
> +    extra=$3
> +    $DEBUGFS -w -R "write /dev/null $name" $TMPFILE > $OUT 2>&1
> +    for xtime in atime ctime mtime crtime
> +    do
> +        $DEBUGFS -w -R "set_inode_field $name $xtime @$time" $TMPFILE > $OUT 2>&1
> +
> +        $DEBUGFS -w -R "set_inode_field $name ${xtime}_extra $extra" $TMPFILE > $OUT 2>&1
> +    done
> +}
> +
> +get_file_xtime_and_extra() {
> +    name=$1
> +    echo "times for $name =" >> $TIMESTAMPS
> +    $DEBUGFS -R "stat $name" $TMPFILE 2>&1 | egrep '^( a| c| m|cr)time: ' |sed 's/ --.*//' >> $TIMESTAMPS
> +}
> +
> +rm -f $OUT
> +rm -f $TIMESTAMPS
> +
> +#create an empty ext4 filesystem with 256-byte inodes for testing
> +dd status=none if=/dev/zero of=$TMPFILE bs=1024 count=5000
> +echo  mkfs.ext4 -q -I 256 $TMPFILE >> $OUT
> +yes | mkfs.ext4 -q -I 256 $TMPFILE >> $OUT 2>&1
> +
> +# this is a pre-1970 file encoded with the old encoding.
> +# fsck should repair this
> +create_file_with_xtime_and_extra year-1909 -1907928000 3
> +
> +# these are all already encoded correctly
> +create_file_with_xtime_and_extra year-1979   299592000 0
> +create_file_with_xtime_and_extra year-2039  2191752000 1
> +create_file_with_xtime_and_extra year-2139  5345352000 1
> +
> +# confirm that the xtime is wrong on the pre-1970 file
> +get_file_xtime_and_extra year-1909
> +
> +# and confirm that it is right on the remaining files
> +get_file_xtime_and_extra year-1979
> +get_file_xtime_and_extra year-2039
> +get_file_xtime_and_extra year-2139
> +
> +# before we repair the filesystem, save off a copy so that
> +# we can use it later
> +
> +cp $TMPFILE $TMPFILE.backup
> +
> +# repair the filesystem
> +E2FSCK_TIME=1386393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
> +
> +# check that the dates and xtime_extra on the file is now correct
> +get_file_xtime_and_extra year-1909
> +
> +# check that the remaining dates have not been altered
> +get_file_xtime_and_extra year-1979
> +get_file_xtime_and_extra year-2039
> +get_file_xtime_and_extra year-2139
> +
> +# now we need to check that after the year 2242, e2fsck does not
> +# modify dates with extra_xtime=3
> +
> +# restore the unrepaired filesystem
> +mv $TMPFILE.backup $TMPFILE
> +
> +#retry the repair
> +E2FSCK_TIME=9270393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
> +
> +# check that the 1909 file is unaltered (i.e. it has a post-2378 date)
> +get_file_xtime_and_extra year-1909
> +
> +cmp -s $TIMESTAMPS $EXP
> +status=$?
> +
> +if [ "$status" = 0 ] ; then
> +    echo "$test_name: $test_description: ok"
> +    touch $test_name.ok
> +else
> +    echo "$test_name: $test_description: failed"
> +    diff $DIFF_OPTS $EXP $TIMESTAMPS > $test_name.failed
> +fi
> +
> +unset OUT TIMESTAMPS EXP FSCK_OPT
> +
> +else #if test -x $DEBUGFS_EXE; then
> +    echo "$test_name: $test_description: skipped"
> +fi
> +
> -- 
> 1.8.1.2
> 
> 
> 

Re: [PATCH v7 1/2] e2fsck: Correct ext4 dates generated by old kernels.

[Theodore Ts'o]

On Sat, Dec 07, 2013 at 03:02:40PM -0500, David Turner wrote:
> 
> However, as Andreas notes, "we want to verify .. that "debugfs -R
> 'stat testfile'" decodes the times correctly."  Unfortunately, it
> does not, and it is not trivial to fix.  debugfs uses an internal
> function time_to_string(__u32), which calls gmtime or localtime.
> These functions are defined on time_t, which is 32-bits on 32-bit
> Linux systems.  In addition, because time_to_string takes
> an unsigned int, it returns bad results for pre-1970 dates.

We can and should change time_to_string to take an unsigned 64-bit
type; it's an internal interface to debugfs.  The question of what to
do if the system time_t is only 32-bit.

What I'd probably suggest is to have a mode (set either by an
environment variable, or a debugfs command) which causes
time_to_string to use an internal version of a 64-bit capable gmtime
function.  This will allow for better regression testing, and it in a
way that will be have stable results regardless of whether a
particular platform, or a future version of glibc has a 64-bit time_t
or not.

> Also, while I was making this change, I happened to notice that
> there is no dtime_extra field into which to put the extra bits.
> This means that there is still a year 2038 problem with tools
> that deal with deleted files (including the corner case that the
> system is shut down cleanly at the exact second that the bottom
> 32 bits of the current time are zero, leading fsck to believe
> that there was an unclean shutdown).

I'm not sure we care, since nothing is actually using the dtime.  This
was originally intended to make it easy to recover from accident file
deletions, using debugfs's lsdel command.  However, ever since ext3,
in order to make sure the file system is consistent if it crashes
during an unlink, we end up truncating the file before we unlink it,
so i_blocks[] is completely cleared for deleted inodes.  This makes it
essentially impossible for lsdel to work.

> I want to make an additional change to correct this.  Since I
> assume it is impossible to add an additional field, I propose to
> use ctime_extra to store the extra bits of dtime on deletion.
> This is a bit hackish, since it does destroy a bit of data, but
> it is significantly better than dtime being totally broken after
> 2038.  What do people think?

Given that we always set ctime when delete a file (which makes sense,
since we are decrementing i_links_count), I think changing debugfs
(which is the only thing that even looks at dtime, BTW) makes a lot of
sense.

Regards,

						- Ted

Re: [PATCH v7 1/2] e2fsck: Correct ext4 dates generated by old kernels.

[David Turner]

On Sat, 2013-12-07 at 19:53 -0500, Theodore Ts'o wrote:
> On Sat, Dec 07, 2013 at 03:02:40PM -0500, David Turner wrote:
> > 
> > However, as Andreas notes, "we want to verify .. that "debugfs -R
> > 'stat testfile'" decodes the times correctly."  Unfortunately, it
> > does not, and it is not trivial to fix.  debugfs uses an internal
> > function time_to_string(__u32), which calls gmtime or localtime.
> > These functions are defined on time_t, which is 32-bits on 32-bit
> > Linux systems.  In addition, because time_to_string takes
> > an unsigned int, it returns bad results for pre-1970 dates.
> 
> We can and should change time_to_string to take an unsigned 64-bit
> type; it's an internal interface to debugfs.  

Shouldn't this be a signed 64-bit type, since we have to support times
before 1970?

> The question of what to do if the system time_t is only 32-bit.
> 
> What I'd probably suggest is to have a mode (set either by an
> environment variable, or a debugfs command) which causes
> time_to_string to use an internal version of a 64-bit capable gmtime
> function.  This will allow for better regression testing, and it in a
> way that will be have stable results regardless of whether a
> particular platform, or a future version of glibc has a 64-bit time_t
> or not.

Presently, the TZ environment variable selects between gmtime and
localtime.  How about the following:

We supply a 64-bit version of gmtime (but not localtime).  If time_t is
32 bits, and the date being printed is after 2038, and TZ is not GMT,
then we return an error message instead of calling localtime.  Then, in
any of the tests that depend on debugfs date printing we can simply set
TZ=GMT, so that they will continue to work regardless of the size of
time_t.

> > Also, while I was making this change, I happened to notice that
> > there is no dtime_extra field into which to put the extra bits.
> > This means that there is still a year 2038 problem with tools
> > that deal with deleted files (including the corner case that the
> > system is shut down cleanly at the exact second that the bottom
> > 32 bits of the current time are zero, leading fsck to believe
> > that there was an unclean shutdown).
> 
> I'm not sure we care, since nothing is actually using the dtime.  This
> was originally intended to make it easy to recover from accident file
> deletions, using debugfs's lsdel command.  However, ever since ext3,
> in order to make sure the file system is consistent if it crashes
> during an unlink, we end up truncating the file before we unlink it,
> so i_blocks[] is completely cleared for deleted inodes.  This makes it
> essentially impossible for lsdel to work.

I just did a quick grep, and it looks like
fs/ext4/ialloc.c:recently_deleted is using it.  That's not relevant to
e2fsprogs, but we should correct it anyway, because it will probably
break in 2038.

Also fs/ext4/inode.c:ext4_do_update_inode -- I'm not sure quite what's
going on there, but it seems like it should be fixed as well.

I'll volunteer to make these changes to the kernel and to e2fsprogs, but
I think my current set of patches can be safely merged before I do that.

> > I want to make an additional change to correct this.  Since I
> > assume it is impossible to add an additional field, I propose to
> > use ctime_extra to store the extra bits of dtime on deletion.
> > This is a bit hackish, since it does destroy a bit of data, but
> > it is significantly better than dtime being totally broken after
> > 2038.  What do people think?
> 
> Given that we always set ctime when delete a file (which makes sense,
> since we are decrementing i_links_count), I think changing debugfs
> (which is the only thing that even looks at dtime, BTW) makes a lot of
> sense.

I'm not really very familiar with the ext4 internals.  Are you saying
that it's safe to just read ctime_extra (without explicitly writing it
when we write dtime)?  Or just that it is safe to overwrite it when we
write dtime? 

Re: [PATCH v7 1/2] e2fsck: Correct ext4 dates generated by old kernels.

[Theodore Ts'o]

On Sat, Dec 07, 2013 at 09:58:17PM -0500, David Turner wrote:
> > We can and should change time_to_string to take an unsigned 64-bit
> > type; it's an internal interface to debugfs.  
> 
> Shouldn't this be a signed 64-bit type, since we have to support times
> before 1970?

That depends on whether we make time_to_string() and string_to_time()
handle the ext4-specific encoding, or whether we make these routines
take an offset in terms of "seconds since the Epoch".  I was assuming
the former, but it does make sense to have ext2fs library functions
which handle the conversion --- but in that case, said library
functions will need to use an explicit 64-bit libext2fs type, since we
can't count on the width of the system-supplied time_t.

> > What I'd probably suggest is to have a mode (set either by an
> > environment variable, or a debugfs command) which causes
> > time_to_string to use an internal version of a 64-bit capable gmtime
> > function.  This will allow for better regression testing, and it in a
> > way that will be have stable results regardless of whether a
> > particular platform, or a future version of glibc has a 64-bit time_t
> > or not.
> 
> Presently, the TZ environment variable selects between gmtime and
> localtime.  How about the following:
> 
> We supply a 64-bit version of gmtime (but not localtime).  If time_t is
> 32 bits, and the date being printed is after 2038, and TZ is not GMT,
> then we return an error message instead of calling localtime.  Then, in
> any of the tests that depend on debugfs date printing we can simply set
> TZ=GMT, so that they will continue to work regardless of the size of
> time_t.

Well, the reason why I wanted a way to explicitly specify the built-in
gmtime was to isolate problems caused by differences in how future
gmtime()'s might be implemented.  Maybe I'm being too paranoid here,
and all of the confusion over leap seconds should be resolved by now.
(e.g., https://groups.google.com/forum/#!topic/comp.unix.programmer/zTXAaa5lnFg)

But we could do this via a special version of TZ (__libext2fs_GMT),
which should be safe since other programs which are using the stock
localtime() function will be fine, since unrecognized TZ values is
interpreted by libc as GMT.

> > Given that we always set ctime when delete a file (which makes sense,
> > since we are decrementing i_links_count), I think changing debugfs
> > (which is the only thing that even looks at dtime, BTW) makes a lot of
> > sense.
> 
> I'm not really very familiar with the ext4 internals.  Are you saying
> that it's safe to just read ctime_extra (without explicitly writing it
> when we write dtime)?  Or just that it is safe to overwrite it when we
> write dtime?

We actually set dtime in ext4_evict_inode(); this gets called when
i_nlink goes to zero, and there are no longer any open file
descriptors referencing the inode.  It is possible for ctime != dtime
if there is still a fd open on the inode at the time of the last
unlink(), and the fd could keep the inode from being actually released
for hours, and in theory, years or decades.

In practice though, the high bits of ctime and dtime should be the
same, unless the above period happens to span one of the encoding
gaps.  The simplest thing to do would be to simply set ctime and dtime
in ext4_evict_inode().  The slightly more complicated thing to do
would be to check to see if the high bits of dtime (if we had a
dtime_extra) are different from ctime_extra, and if so, in that case,
set ctime and dtime.

It doesn't really matter since by the time the inode is unlinked,
nothing else will see the contents of the inode except for a file
system developer who is poking at the file system using debugfs.  It
might be useful in some circumstances to see when the file was
unlinked versus when it was actually finally relased, but that's a
pretty minor edge case.

Regards,

						- Ted

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[Darrick J. Wong]

On Mon, Nov 11, 2013 at 07:30:18PM -0500, Theodore Ts'o wrote:
> On Sun, Nov 10, 2013 at 02:56:54AM -0500, David Turner wrote:
> > b. Use Andreas's encoding, which is incompatible with pre-1970 files
> > written on 64-bit systems.
> >
> > I don't care about currently-existing post-2038 files, because I believe
> > that nobody has a valid reason to have such files.  However, I do
> > believe that pre-1970 files are probably important to someone.
> > 
> > Despite this, I prefer option (b), because I think the simplicity is
> > valuable, and because I hate to give up date ranges (even ones that I
> > think we'll "never" need). Option (b) is not actually lossy, because we
> > could correct pre-1970 files with e2fsck; under Andreas's encoding,
> > their dates would be in the far future (and thus cannot be legitimate).
> > 
> > Would a patch that does (b) be accepted?  I would accompany it with a
> > patch to e2fsck (which I assume would also go to the ext4 developers
> > mailing list?).
> 
> I agree, I think this is the best way to go.  I'm going to drop your
> earlier patch, and wait for an updated patch from you.  It may miss
> this merge window, but as Andreas has pointed out, we still have a few
> years to get this right.  :-)

Just out of curiosity, did this (updated patch) ever happen?

--D
> 
> Thanks!!
> 
> 					- 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

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[David Turner]

On Tue, 2014-01-21 at 22:22 -0800, Darrick J. Wong wrote:
> On Mon, Nov 11, 2013 at 07:30:18PM -0500, Theodore Ts'o wrote:
> > On Sun, Nov 10, 2013 at 02:56:54AM -0500, David Turner wrote:
> > > b. Use Andreas's encoding, which is incompatible with pre-1970 files
> > > written on 64-bit systems.
> > >
> > > I don't care about currently-existing post-2038 files, because I believe
> > > that nobody has a valid reason to have such files.  However, I do
> > > believe that pre-1970 files are probably important to someone.
> > > 
> > > Despite this, I prefer option (b), because I think the simplicity is
> > > valuable, and because I hate to give up date ranges (even ones that I
> > > think we'll "never" need). Option (b) is not actually lossy, because we
> > > could correct pre-1970 files with e2fsck; under Andreas's encoding,
> > > their dates would be in the far future (and thus cannot be legitimate).
> > > 
> > > Would a patch that does (b) be accepted?  I would accompany it with a
> > > patch to e2fsck (which I assume would also go to the ext4 developers
> > > mailing list?).
> > 
> > I agree, I think this is the best way to go.  I'm going to drop your
> > earlier patch, and wait for an updated patch from you.  It may miss
> > this merge window, but as Andreas has pointed out, we still have a few
> > years to get this right.  :-)
> 
> Just out of curiosity, did this (updated patch) ever happen?

I think I sent a usable patch that Ted merged part of into e2fscktools;
the kernel portion was dropped for some reason.

While I was waiting to hear back on the kernel portion, I started
looking into the dtime stuff, but then I got distracted by a new job.

Assuming that I won't have time to deal with dtime (since it seems to be
much more complicated), is the right way forward for me to rebase the
non-dtime portion of my patch against the latest kernel, and resend it?
If so, will it get merged?  (Assume here that I do the same with the
e2fsck stuff)

Re: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2712 bytes --]

On Feb 10, 2014, at 10:12 PM, David Turner <novalis@novalis.org> wrote:
> On Tue, 2014-01-21 at 22:22 -0800, Darrick J. Wong wrote:
>> On Mon, Nov 11, 2013 at 07:30:18PM -0500, Theodore Ts'o wrote:
>>> On Sun, Nov 10, 2013 at 02:56:54AM -0500, David Turner wrote:
>>>> b. Use Andreas's encoding, which is incompatible with pre-1970 files
>>>> written on 64-bit systems.
>>>> 
>>>> I don't care about currently-existing post-2038 files, because I believe
>>>> that nobody has a valid reason to have such files.  However, I do
>>>> believe that pre-1970 files are probably important to someone.
>>>> 
>>>> Despite this, I prefer option (b), because I think the simplicity is
>>>> valuable, and because I hate to give up date ranges (even ones that I
>>>> think we'll "never" need). Option (b) is not actually lossy, because we could correct pre-1970 files with e2fsck; under Andreas's encoding,
>>>> their dates would be in the far future (and thus cannot be legitimate).
>>>> 
>>>> Would a patch that does (b) be accepted?  I would accompany it with a
>>>> patch to e2fsck (which I assume would also go to the ext4 developers
>>>> mailing list?).
>>> 
>>> I agree, I think this is the best way to go.  I'm going to drop your
>>> earlier patch, and wait for an updated patch from you.  It may miss
>>> this merge window, but as Andreas has pointed out, we still have a few
>>> years to get this right.  :-)
>> 
>> Just out of curiosity, did this (updated patch) ever happen?
> 
> I think I sent a usable patch that Ted merged part of into e2fscktools;
> the kernel portion was dropped for some reason.
> 
> While I was waiting to hear back on the kernel portion, I started
> looking into the dtime stuff, but then I got distracted by a new job.
> 
> Assuming that I won't have time to deal with dtime (since it seems to be
> much more complicated), is the right way forward for me to rebase the
> non-dtime portion of my patch against the latest kernel, and resend it?
> If so, will it get merged?  (Assume here that I do the same with the
> e2fsck stuff)

Yes, please submit an updated patch for the kernel.  Ted will hopefully
merge it this time.

I don't know that we really care about dtime so much, since it is mostly
just treated as "zero == not deleted" and "non-zero == deleted" by e2fsck,
and maybe useful for manual debugging.  We might consider that it uses a
sliding window in the future, since I'm sure we won't care about files
deleted more than 70 years ago, and if we did the fact that they appear
to be files deleted 70 years in the future won't matter so much.  In any
case, that can be looked at in a separate patch.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v8 1/2] ext4: Fix handling of extended tv_sec (bug 23732)

[David Turner]

against tytso/dev

--
In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
the {a,c,m}time fields, deferring the year 2038 problem to the year
2446.

When decoding these extended fields, for times whose bottom 32 bits
would represent a negative number, sign extension causes the 64-bit
extended timestamp to be negative as well, which is not what's
intended.  This patch corrects that issue, so that the only negative
{a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
timestamps).

Some older kernels might have written pre-1970 dates with 1,1 in the
extra bits.  This patch treats those incorrectly-encoded dates as
pre-1970, instead of post-2311, until kernel 4.20 is released.
Hopefully by then e2fsck will have fixed up the bad data.

Also add a comment explaining the encoding of ext4's extra {a,c,m}time
bits.

Signed-off-by: David Turner <novalis@novalis.org>
Reported-by: Mark Harris <mh8928@yahoo.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
---
 fs/ext4/ext4.h | 49 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index ece5556..f9c6d2f 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -26,6 +26,7 @@
 #include <linux/seqlock.h>
 #include <linux/mutex.h>
 #include <linux/timer.h>
+#include <linux/version.h>
 #include <linux/wait.h>
 #include <linux/blockgroup_lock.h>
 #include <linux/percpu_counter.h>
@@ -724,19 +725,53 @@ struct move_extent {
 	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
 	    (einode)->i_extra_isize))			\
 
+/*
+ * We need is an encoding that preserves the times for extra epoch
"00":
+ *
+ * extra  msb of                         adjust for signed
+ * epoch  32-bit                         32-bit tv_sec to
+ * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time
range
+ * 0 0    1    -0x80000000..-0x00000001  0x000000000
1901-12-13..1969-12-31
+ * 0 0    0    0x000000000..0x07fffffff  0x000000000
1970-01-01..2038-01-19
+ * 0 1    1    0x080000000..0x0ffffffff  0x100000000
2038-01-19..2106-02-07
+ * 0 1    0    0x100000000..0x17fffffff  0x100000000
2106-02-07..2174-02-25
+ * 1 0    1    0x180000000..0x1ffffffff  0x200000000
2174-02-25..2242-03-16
+ * 1 0    0    0x200000000..0x27fffffff  0x200000000
2242-03-16..2310-04-04
+ * 1 1    1    0x280000000..0x2ffffffff  0x300000000
2310-04-04..2378-04-22
+ * 1 1    0    0x300000000..0x37fffffff  0x300000000
2378-04-22..2446-05-10
+ *
+ * Note that previous versions of the kernel on 64-bit systems would
+ * incorrectly use extra epoch bits 1,1 for dates between 1901 and
+ * 1970.  e2fsck will correct this, assuming that it is run on the
+ * affected filesystem before 2242.
+ */
+
 static inline __le32 ext4_encode_extra_time(struct timespec *time)
 {
-       return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
-			   (time->tv_sec >> 32) & EXT4_EPOCH_MASK : 0) |
-                          ((time->tv_nsec << EXT4_EPOCH_BITS) &
EXT4_NSEC_MASK));
+	u32 extra = sizeof(time->tv_sec) > 4 ?
+		((time->tv_sec - (s32)time->tv_sec) >> 32) & EXT4_EPOCH_MASK : 0;
+	return cpu_to_le32(extra | (time->tv_nsec << EXT4_EPOCH_BITS));
 }
 
 static inline void ext4_decode_extra_time(struct timespec *time, __le32
extra)
 {
-       if (sizeof(time->tv_sec) > 4)
-	       time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
-			       << 32;
-       time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
EXT4_EPOCH_BITS;
+	if (unlikely(sizeof(time->tv_sec) > 4 &&
+			(extra & cpu_to_le32(EXT4_EPOCH_MASK)))) {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,20,0)
+		/* Handle legacy encoding of pre-1970 dates with epoch
+		 * bits 1,1.  We assume that by kernel version 4.20,
+		 * everyone will have run fsck over the affected
+		 * filesystems to correct the problem.
+		 */
+		u64 extra_bits = le32_to_cpu(extra) & EXT4_EPOCH_MASK;
+		if (extra_bits == 3)
+			extra_bits = 0;
+		time->tv_sec += extra_bits << 32;
+#else
+		time->tv_sec += (u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;
+#endif
+	}
+	time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
EXT4_EPOCH_BITS;
 }
 
 #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)			       \
-- 
1.8.1.2

[PATCH v8 2/2] e2fsck: Correct ext4 dates generated by old kernels

[David Turner]

against e2fsprogs/next
--
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).

Includes tests for this, as well as changes to debugfs to correctly
set crtimes.

Signed-off-by: David Turner <novalis@novalis.org>
---
 debugfs/set_fields.c                  |  2 +-
 e2fsck/pass1.c                        | 43 ++++++++++++++++
 e2fsck/problem.c                      |  4 ++
 e2fsck/problem.h                      |  4 ++
 lib/extra_epoch.h                     |  2 +
 tests/f_pre_1970_date_encoding/expect | 45 ++++++++++++++++
 tests/f_pre_1970_date_encoding/name   |  1 +
 tests/f_pre_1970_date_encoding/script | 96
+++++++++++++++++++++++++++++++++++
 8 files changed, 196 insertions(+), 1 deletion(-)
 create mode 100644 lib/extra_epoch.h
 create mode 100644 tests/f_pre_1970_date_encoding/expect
 create mode 100644 tests/f_pre_1970_date_encoding/name
 create mode 100644 tests/f_pre_1970_date_encoding/script

diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
index aad1cd8..f7c55a7 100644
--- a/debugfs/set_fields.c
+++ b/debugfs/set_fields.c
@@ -200,7 +200,7 @@ static struct field_set_info inode_fields[] = {
 		4, parse_uint },
 	{ "atime_extra", &set_inode.i_atime_extra, NULL,
 		4, parse_uint },
-	{ "crtime", &set_inode.i_crtime, NULL, 4, parse_uint },
+	{ "crtime", &set_inode.i_crtime, NULL, 4, parse_time },
 	{ "crtime_extra", &set_inode.i_crtime_extra, NULL,
 		4, parse_uint },
 	{ "bmap", NULL, NULL, 4, parse_bmap, FLAG_ARRAY },
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index ab23e42..ecbd79e 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -50,6 +50,8 @@
 
 #include "problem.h"
 
+#include "extra_epoch.h"
+
 #ifdef NO_INLINE_FUNCS
 #define _INLINE_
 #else
@@ -348,6 +350,21 @@ fix:
 				EXT2_INODE_SIZE(sb), "pass1");
 }
 
+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, crtimes, 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 +405,32 @@ 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, crtime, 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, crtime) ||
+	     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, crtime))
+			inode->i_crtime_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
  */
diff --git a/lib/extra_epoch.h b/lib/extra_epoch.h
new file mode 100644
index 0000000..465c43f
--- /dev/null
+++ b/lib/extra_epoch.h
@@ -0,0 +1,2 @@
+#define EXT4_EPOCH_BITS 2
+#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
diff --git a/tests/f_pre_1970_date_encoding/expect
b/tests/f_pre_1970_date_encoding/expect
new file mode 100644
index 0000000..1a71571
--- /dev/null
+++ b/tests/f_pre_1970_date_encoding/expect
@@ -0,0 +1,45 @@
+times for year-1909 =
+ ctime: 0x8e475440:00000003
+ atime: 0x8e475440:00000003
+ mtime: 0x8e475440:00000003
+crtime: 0x8e475440:00000003
+times for year-1979 =
+ ctime: 0x11db6940:00000000
+ atime: 0x11db6940:00000000
+ mtime: 0x11db6940:00000000
+crtime: 0x11db6940:00000000
+times for year-2039 =
+ ctime: 0x82a37b40:00000001
+ atime: 0x82a37b40:00000001
+ mtime: 0x82a37b40:00000001
+crtime: 0x82a37b40:00000001
+times for year-2139 =
+ ctime: 0x3e9b9940:00000001
+ atime: 0x3e9b9940:00000001
+ mtime: 0x3e9b9940:00000001
+crtime: 0x3e9b9940:00000001
+times for year-1909 =
+ ctime: 0x8e475440:00000000
+ atime: 0x8e475440:00000000
+ mtime: 0x8e475440:00000000
+crtime: 0x8e475440:00000000
+times for year-1979 =
+ ctime: 0x11db6940:00000000
+ atime: 0x11db6940:00000000
+ mtime: 0x11db6940:00000000
+crtime: 0x11db6940:00000000
+times for year-2039 =
+ ctime: 0x82a37b40:00000001
+ atime: 0x82a37b40:00000001
+ mtime: 0x82a37b40:00000001
+crtime: 0x82a37b40:00000001
+times for year-2139 =
+ ctime: 0x3e9b9940:00000001
+ atime: 0x3e9b9940:00000001
+ mtime: 0x3e9b9940:00000001
+crtime: 0x3e9b9940:00000001
+times for year-1909 =
+ ctime: 0x8e475440:00000003
+ atime: 0x8e475440:00000003
+ mtime: 0x8e475440:00000003
+crtime: 0x8e475440:00000003
diff --git a/tests/f_pre_1970_date_encoding/name
b/tests/f_pre_1970_date_encoding/name
new file mode 100644
index 0000000..9805324
--- /dev/null
+++ b/tests/f_pre_1970_date_encoding/name
@@ -0,0 +1 @@
+correct mis-encoded pre-1970 dates
diff --git a/tests/f_pre_1970_date_encoding/script
b/tests/f_pre_1970_date_encoding/script
new file mode 100644
index 0000000..c3e12f5
--- /dev/null
+++ b/tests/f_pre_1970_date_encoding/script
@@ -0,0 +1,96 @@
+if test -x $DEBUGFS_EXE; then
+
+OUT=$test_name.log
+TIMESTAMPS=$test_name.timestamps.log
+EXP=$test_dir/expect
+FSCK_OPT=-yf
+
+create_file_with_xtime_and_extra() {
+    name=$1
+    time=$2
+    extra=$3
+    $DEBUGFS -w -R "write /dev/null $name" $TMPFILE > $OUT 2>&1
+    for xtime in atime ctime mtime crtime
+    do
+        $DEBUGFS -w -R "set_inode_field $name $xtime @$time" $TMPFILE >
$OUT 2>&1
+
+        $DEBUGFS -w -R "set_inode_field $name ${xtime}_extra $extra"
$TMPFILE > $OUT 2>&1
+    done
+}
+
+get_file_xtime_and_extra() {
+    name=$1
+    echo "times for $name =" >> $TIMESTAMPS
+    $DEBUGFS -R "stat $name" $TMPFILE 2>&1 | egrep '^( a| c| m|cr)time:
' |sed 's/ --.*//' >> $TIMESTAMPS
+}
+
+rm -f $OUT
+rm -f $TIMESTAMPS
+
+#create an empty ext4 filesystem with 256-byte inodes for testing
+dd status=none if=/dev/zero of=$TMPFILE bs=1024 count=5000
+echo  mkfs.ext4 -q -I 256 $TMPFILE >> $OUT
+yes | mkfs.ext4 -q -I 256 $TMPFILE >> $OUT 2>&1
+
+# this is a pre-1970 file encoded with the old encoding.
+# fsck should repair this
+create_file_with_xtime_and_extra year-1909 -1907928000 3
+
+# these are all already encoded correctly
+create_file_with_xtime_and_extra year-1979   299592000 0
+create_file_with_xtime_and_extra year-2039  2191752000 1
+create_file_with_xtime_and_extra year-2139  5345352000 1
+
+# confirm that the xtime is wrong on the pre-1970 file
+get_file_xtime_and_extra year-1909
+
+# and confirm that it is right on the remaining files
+get_file_xtime_and_extra year-1979
+get_file_xtime_and_extra year-2039
+get_file_xtime_and_extra year-2139
+
+# before we repair the filesystem, save off a copy so that
+# we can use it later
+
+cp $TMPFILE $TMPFILE.backup
+
+# repair the filesystem
+E2FSCK_TIME=1386393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
+
+# check that the dates and xtime_extra on the file is now correct
+get_file_xtime_and_extra year-1909
+
+# check that the remaining dates have not been altered
+get_file_xtime_and_extra year-1979
+get_file_xtime_and_extra year-2039
+get_file_xtime_and_extra year-2139
+
+# now we need to check that after the year 2242, e2fsck does not
+# modify dates with extra_xtime=3
+
+# restore the unrepaired filesystem
+mv $TMPFILE.backup $TMPFILE
+
+#retry the repair
+E2FSCK_TIME=9270393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
+
+# check that the 1909 file is unaltered (i.e. it has a post-2378 date)
+get_file_xtime_and_extra year-1909
+
+cmp -s $TIMESTAMPS $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $TIMESTAMPS > $test_name.failed
+fi
+
+unset OUT TIMESTAMPS EXP FSCK_OPT
+
+else #if test -x $DEBUGFS_EXE; then
+	echo "$test_name: $test_description: skipped"
+fi
+
-- 
1.8.1.2

Re: [PATCH v8 2/2] e2fsck: Correct ext4 dates generated by old kernels

[Andreas Dilger]

Thanks for the patch, and especially the good test case. You (or Ted) can add:

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

for both of these patches. 

Cheers, Andreas

> On Feb 13, 2014, at 20:47, David Turner <novalis@novalis.org> wrote:
> 
> against e2fsprogs/next
> --
> 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).
> 
> Includes tests for this, as well as changes to debugfs to correctly
> set crtimes.
> 
> Signed-off-by: David Turner <novalis@novalis.org>
> ---
> debugfs/set_fields.c                  |  2 +-
> e2fsck/pass1.c                        | 43 ++++++++++++++++
> e2fsck/problem.c                      |  4 ++
> e2fsck/problem.h                      |  4 ++
> lib/extra_epoch.h                     |  2 +
> tests/f_pre_1970_date_encoding/expect | 45 ++++++++++++++++
> tests/f_pre_1970_date_encoding/name   |  1 +
> tests/f_pre_1970_date_encoding/script | 96
> +++++++++++++++++++++++++++++++++++
> 8 files changed, 196 insertions(+), 1 deletion(-)
> create mode 100644 lib/extra_epoch.h
> create mode 100644 tests/f_pre_1970_date_encoding/expect
> create mode 100644 tests/f_pre_1970_date_encoding/name
> create mode 100644 tests/f_pre_1970_date_encoding/script
> 
> diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
> index aad1cd8..f7c55a7 100644
> --- a/debugfs/set_fields.c
> +++ b/debugfs/set_fields.c
> @@ -200,7 +200,7 @@ static struct field_set_info inode_fields[] = {
>        4, parse_uint },
>    { "atime_extra", &set_inode.i_atime_extra, NULL,
>        4, parse_uint },
> -    { "crtime", &set_inode.i_crtime, NULL, 4, parse_uint },
> +    { "crtime", &set_inode.i_crtime, NULL, 4, parse_time },
>    { "crtime_extra", &set_inode.i_crtime_extra, NULL,
>        4, parse_uint },
>    { "bmap", NULL, NULL, 4, parse_bmap, FLAG_ARRAY },
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index ab23e42..ecbd79e 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -50,6 +50,8 @@
> 
> #include "problem.h"
> 
> +#include "extra_epoch.h"
> +
> #ifdef NO_INLINE_FUNCS
> #define _INLINE_
> #else
> @@ -348,6 +350,21 @@ fix:
>                EXT2_INODE_SIZE(sb), "pass1");
> }
> 
> +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, crtimes, 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 +405,32 @@ 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, crtime, 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, crtime) ||
> +         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, crtime))
> +            inode->i_crtime_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
>  */
> diff --git a/lib/extra_epoch.h b/lib/extra_epoch.h
> new file mode 100644
> index 0000000..465c43f
> --- /dev/null
> +++ b/lib/extra_epoch.h
> @@ -0,0 +1,2 @@
> +#define EXT4_EPOCH_BITS 2
> +#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
> diff --git a/tests/f_pre_1970_date_encoding/expect
> b/tests/f_pre_1970_date_encoding/expect
> new file mode 100644
> index 0000000..1a71571
> --- /dev/null
> +++ b/tests/f_pre_1970_date_encoding/expect
> @@ -0,0 +1,45 @@
> +times for year-1909 =
> + ctime: 0x8e475440:00000003
> + atime: 0x8e475440:00000003
> + mtime: 0x8e475440:00000003
> +crtime: 0x8e475440:00000003
> +times for year-1979 =
> + ctime: 0x11db6940:00000000
> + atime: 0x11db6940:00000000
> + mtime: 0x11db6940:00000000
> +crtime: 0x11db6940:00000000
> +times for year-2039 =
> + ctime: 0x82a37b40:00000001
> + atime: 0x82a37b40:00000001
> + mtime: 0x82a37b40:00000001
> +crtime: 0x82a37b40:00000001
> +times for year-2139 =
> + ctime: 0x3e9b9940:00000001
> + atime: 0x3e9b9940:00000001
> + mtime: 0x3e9b9940:00000001
> +crtime: 0x3e9b9940:00000001
> +times for year-1909 =
> + ctime: 0x8e475440:00000000
> + atime: 0x8e475440:00000000
> + mtime: 0x8e475440:00000000
> +crtime: 0x8e475440:00000000
> +times for year-1979 =
> + ctime: 0x11db6940:00000000
> + atime: 0x11db6940:00000000
> + mtime: 0x11db6940:00000000
> +crtime: 0x11db6940:00000000
> +times for year-2039 =
> + ctime: 0x82a37b40:00000001
> + atime: 0x82a37b40:00000001
> + mtime: 0x82a37b40:00000001
> +crtime: 0x82a37b40:00000001
> +times for year-2139 =
> + ctime: 0x3e9b9940:00000001
> + atime: 0x3e9b9940:00000001
> + mtime: 0x3e9b9940:00000001
> +crtime: 0x3e9b9940:00000001
> +times for year-1909 =
> + ctime: 0x8e475440:00000003
> + atime: 0x8e475440:00000003
> + mtime: 0x8e475440:00000003
> +crtime: 0x8e475440:00000003
> diff --git a/tests/f_pre_1970_date_encoding/name
> b/tests/f_pre_1970_date_encoding/name
> new file mode 100644
> index 0000000..9805324
> --- /dev/null
> +++ b/tests/f_pre_1970_date_encoding/name
> @@ -0,0 +1 @@
> +correct mis-encoded pre-1970 dates
> diff --git a/tests/f_pre_1970_date_encoding/script
> b/tests/f_pre_1970_date_encoding/script
> new file mode 100644
> index 0000000..c3e12f5
> --- /dev/null
> +++ b/tests/f_pre_1970_date_encoding/script
> @@ -0,0 +1,96 @@
> +if test -x $DEBUGFS_EXE; then
> +
> +OUT=$test_name.log
> +TIMESTAMPS=$test_name.timestamps.log
> +EXP=$test_dir/expect
> +FSCK_OPT=-yf
> +
> +create_file_with_xtime_and_extra() {
> +    name=$1
> +    time=$2
> +    extra=$3
> +    $DEBUGFS -w -R "write /dev/null $name" $TMPFILE > $OUT 2>&1
> +    for xtime in atime ctime mtime crtime
> +    do
> +        $DEBUGFS -w -R "set_inode_field $name $xtime @$time" $TMPFILE >
> $OUT 2>&1
> +
> +        $DEBUGFS -w -R "set_inode_field $name ${xtime}_extra $extra"
> $TMPFILE > $OUT 2>&1
> +    done
> +}
> +
> +get_file_xtime_and_extra() {
> +    name=$1
> +    echo "times for $name =" >> $TIMESTAMPS
> +    $DEBUGFS -R "stat $name" $TMPFILE 2>&1 | egrep '^( a| c| m|cr)time:
> ' |sed 's/ --.*//' >> $TIMESTAMPS
> +}
> +
> +rm -f $OUT
> +rm -f $TIMESTAMPS
> +
> +#create an empty ext4 filesystem with 256-byte inodes for testing
> +dd status=none if=/dev/zero of=$TMPFILE bs=1024 count=5000
> +echo  mkfs.ext4 -q -I 256 $TMPFILE >> $OUT
> +yes | mkfs.ext4 -q -I 256 $TMPFILE >> $OUT 2>&1
> +
> +# this is a pre-1970 file encoded with the old encoding.
> +# fsck should repair this
> +create_file_with_xtime_and_extra year-1909 -1907928000 3
> +
> +# these are all already encoded correctly
> +create_file_with_xtime_and_extra year-1979   299592000 0
> +create_file_with_xtime_and_extra year-2039  2191752000 1
> +create_file_with_xtime_and_extra year-2139  5345352000 1
> +
> +# confirm that the xtime is wrong on the pre-1970 file
> +get_file_xtime_and_extra year-1909
> +
> +# and confirm that it is right on the remaining files
> +get_file_xtime_and_extra year-1979
> +get_file_xtime_and_extra year-2039
> +get_file_xtime_and_extra year-2139
> +
> +# before we repair the filesystem, save off a copy so that
> +# we can use it later
> +
> +cp $TMPFILE $TMPFILE.backup
> +
> +# repair the filesystem
> +E2FSCK_TIME=1386393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
> +
> +# check that the dates and xtime_extra on the file is now correct
> +get_file_xtime_and_extra year-1909
> +
> +# check that the remaining dates have not been altered
> +get_file_xtime_and_extra year-1979
> +get_file_xtime_and_extra year-2039
> +get_file_xtime_and_extra year-2139
> +
> +# now we need to check that after the year 2242, e2fsck does not
> +# modify dates with extra_xtime=3
> +
> +# restore the unrepaired filesystem
> +mv $TMPFILE.backup $TMPFILE
> +
> +#retry the repair
> +E2FSCK_TIME=9270393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
> +
> +# check that the 1909 file is unaltered (i.e. it has a post-2378 date)
> +get_file_xtime_and_extra year-1909
> +
> +cmp -s $TIMESTAMPS $EXP
> +status=$?
> +
> +if [ "$status" = 0 ] ; then
> +    echo "$test_name: $test_description: ok"
> +    touch $test_name.ok
> +else
> +    echo "$test_name: $test_description: failed"
> +    diff $DIFF_OPTS $EXP $TIMESTAMPS > $test_name.failed
> +fi
> +
> +unset OUT TIMESTAMPS EXP FSCK_OPT
> +
> +else #if test -x $DEBUGFS_EXE; then
> +    echo "$test_name: $test_description: skipped"
> +fi
> +
> -- 
> 1.8.1.2
> 
> 
> 

Re: [PATCH v8 2/2] e2fsck: Correct ext4 dates generated by old kernels

[Darrick J. Wong]

Looks good to me too, so you can also add
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

On Thu, Feb 13, 2014 at 10:40:06PM -0700, Andreas Dilger wrote:
> Thanks for the patch, and especially the good test case. You (or Ted) can add:
> 
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> 
> for both of these patches. 
> 
> Cheers, Andreas
> 
> > On Feb 13, 2014, at 20:47, David Turner <novalis@novalis.org> wrote:
> > 
> > against e2fsprogs/next
> > --
> > 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).
> > 
> > Includes tests for this, as well as changes to debugfs to correctly
> > set crtimes.
> > 
> > Signed-off-by: David Turner <novalis@novalis.org>
> > ---
> > debugfs/set_fields.c                  |  2 +-
> > e2fsck/pass1.c                        | 43 ++++++++++++++++
> > e2fsck/problem.c                      |  4 ++
> > e2fsck/problem.h                      |  4 ++
> > lib/extra_epoch.h                     |  2 +
> > tests/f_pre_1970_date_encoding/expect | 45 ++++++++++++++++
> > tests/f_pre_1970_date_encoding/name   |  1 +
> > tests/f_pre_1970_date_encoding/script | 96
> > +++++++++++++++++++++++++++++++++++
> > 8 files changed, 196 insertions(+), 1 deletion(-)
> > create mode 100644 lib/extra_epoch.h
> > create mode 100644 tests/f_pre_1970_date_encoding/expect
> > create mode 100644 tests/f_pre_1970_date_encoding/name
> > create mode 100644 tests/f_pre_1970_date_encoding/script
> > 
> > diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
> > index aad1cd8..f7c55a7 100644
> > --- a/debugfs/set_fields.c
> > +++ b/debugfs/set_fields.c
> > @@ -200,7 +200,7 @@ static struct field_set_info inode_fields[] = {
> >        4, parse_uint },
> >    { "atime_extra", &set_inode.i_atime_extra, NULL,
> >        4, parse_uint },
> > -    { "crtime", &set_inode.i_crtime, NULL, 4, parse_uint },
> > +    { "crtime", &set_inode.i_crtime, NULL, 4, parse_time },
> >    { "crtime_extra", &set_inode.i_crtime_extra, NULL,
> >        4, parse_uint },
> >    { "bmap", NULL, NULL, 4, parse_bmap, FLAG_ARRAY },
> > diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> > index ab23e42..ecbd79e 100644
> > --- a/e2fsck/pass1.c
> > +++ b/e2fsck/pass1.c
> > @@ -50,6 +50,8 @@
> > 
> > #include "problem.h"
> > 
> > +#include "extra_epoch.h"
> > +
> > #ifdef NO_INLINE_FUNCS
> > #define _INLINE_
> > #else
> > @@ -348,6 +350,21 @@ fix:
> >                EXT2_INODE_SIZE(sb), "pass1");
> > }
> > 
> > +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, crtimes, 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 +405,32 @@ 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, crtime, 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, crtime) ||
> > +         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, crtime))
> > +            inode->i_crtime_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
> >  */
> > diff --git a/lib/extra_epoch.h b/lib/extra_epoch.h
> > new file mode 100644
> > index 0000000..465c43f
> > --- /dev/null
> > +++ b/lib/extra_epoch.h
> > @@ -0,0 +1,2 @@
> > +#define EXT4_EPOCH_BITS 2
> > +#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
> > diff --git a/tests/f_pre_1970_date_encoding/expect
> > b/tests/f_pre_1970_date_encoding/expect
> > new file mode 100644
> > index 0000000..1a71571
> > --- /dev/null
> > +++ b/tests/f_pre_1970_date_encoding/expect
> > @@ -0,0 +1,45 @@
> > +times for year-1909 =
> > + ctime: 0x8e475440:00000003
> > + atime: 0x8e475440:00000003
> > + mtime: 0x8e475440:00000003
> > +crtime: 0x8e475440:00000003
> > +times for year-1979 =
> > + ctime: 0x11db6940:00000000
> > + atime: 0x11db6940:00000000
> > + mtime: 0x11db6940:00000000
> > +crtime: 0x11db6940:00000000
> > +times for year-2039 =
> > + ctime: 0x82a37b40:00000001
> > + atime: 0x82a37b40:00000001
> > + mtime: 0x82a37b40:00000001
> > +crtime: 0x82a37b40:00000001
> > +times for year-2139 =
> > + ctime: 0x3e9b9940:00000001
> > + atime: 0x3e9b9940:00000001
> > + mtime: 0x3e9b9940:00000001
> > +crtime: 0x3e9b9940:00000001
> > +times for year-1909 =
> > + ctime: 0x8e475440:00000000
> > + atime: 0x8e475440:00000000
> > + mtime: 0x8e475440:00000000
> > +crtime: 0x8e475440:00000000
> > +times for year-1979 =
> > + ctime: 0x11db6940:00000000
> > + atime: 0x11db6940:00000000
> > + mtime: 0x11db6940:00000000
> > +crtime: 0x11db6940:00000000
> > +times for year-2039 =
> > + ctime: 0x82a37b40:00000001
> > + atime: 0x82a37b40:00000001
> > + mtime: 0x82a37b40:00000001
> > +crtime: 0x82a37b40:00000001
> > +times for year-2139 =
> > + ctime: 0x3e9b9940:00000001
> > + atime: 0x3e9b9940:00000001
> > + mtime: 0x3e9b9940:00000001
> > +crtime: 0x3e9b9940:00000001
> > +times for year-1909 =
> > + ctime: 0x8e475440:00000003
> > + atime: 0x8e475440:00000003
> > + mtime: 0x8e475440:00000003
> > +crtime: 0x8e475440:00000003
> > diff --git a/tests/f_pre_1970_date_encoding/name
> > b/tests/f_pre_1970_date_encoding/name
> > new file mode 100644
> > index 0000000..9805324
> > --- /dev/null
> > +++ b/tests/f_pre_1970_date_encoding/name
> > @@ -0,0 +1 @@
> > +correct mis-encoded pre-1970 dates
> > diff --git a/tests/f_pre_1970_date_encoding/script
> > b/tests/f_pre_1970_date_encoding/script
> > new file mode 100644
> > index 0000000..c3e12f5
> > --- /dev/null
> > +++ b/tests/f_pre_1970_date_encoding/script
> > @@ -0,0 +1,96 @@
> > +if test -x $DEBUGFS_EXE; then
> > +
> > +OUT=$test_name.log
> > +TIMESTAMPS=$test_name.timestamps.log
> > +EXP=$test_dir/expect
> > +FSCK_OPT=-yf
> > +
> > +create_file_with_xtime_and_extra() {
> > +    name=$1
> > +    time=$2
> > +    extra=$3
> > +    $DEBUGFS -w -R "write /dev/null $name" $TMPFILE > $OUT 2>&1
> > +    for xtime in atime ctime mtime crtime
> > +    do
> > +        $DEBUGFS -w -R "set_inode_field $name $xtime @$time" $TMPFILE >
> > $OUT 2>&1
> > +
> > +        $DEBUGFS -w -R "set_inode_field $name ${xtime}_extra $extra"
> > $TMPFILE > $OUT 2>&1
> > +    done
> > +}
> > +
> > +get_file_xtime_and_extra() {
> > +    name=$1
> > +    echo "times for $name =" >> $TIMESTAMPS
> > +    $DEBUGFS -R "stat $name" $TMPFILE 2>&1 | egrep '^( a| c| m|cr)time:
> > ' |sed 's/ --.*//' >> $TIMESTAMPS
> > +}
> > +
> > +rm -f $OUT
> > +rm -f $TIMESTAMPS
> > +
> > +#create an empty ext4 filesystem with 256-byte inodes for testing
> > +dd status=none if=/dev/zero of=$TMPFILE bs=1024 count=5000
> > +echo  mkfs.ext4 -q -I 256 $TMPFILE >> $OUT
> > +yes | mkfs.ext4 -q -I 256 $TMPFILE >> $OUT 2>&1
> > +
> > +# this is a pre-1970 file encoded with the old encoding.
> > +# fsck should repair this
> > +create_file_with_xtime_and_extra year-1909 -1907928000 3
> > +
> > +# these are all already encoded correctly
> > +create_file_with_xtime_and_extra year-1979   299592000 0
> > +create_file_with_xtime_and_extra year-2039  2191752000 1
> > +create_file_with_xtime_and_extra year-2139  5345352000 1
> > +
> > +# confirm that the xtime is wrong on the pre-1970 file
> > +get_file_xtime_and_extra year-1909
> > +
> > +# and confirm that it is right on the remaining files
> > +get_file_xtime_and_extra year-1979
> > +get_file_xtime_and_extra year-2039
> > +get_file_xtime_and_extra year-2139
> > +
> > +# before we repair the filesystem, save off a copy so that
> > +# we can use it later
> > +
> > +cp $TMPFILE $TMPFILE.backup
> > +
> > +# repair the filesystem
> > +E2FSCK_TIME=1386393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
> > +
> > +# check that the dates and xtime_extra on the file is now correct
> > +get_file_xtime_and_extra year-1909
> > +
> > +# check that the remaining dates have not been altered
> > +get_file_xtime_and_extra year-1979
> > +get_file_xtime_and_extra year-2039
> > +get_file_xtime_and_extra year-2139
> > +
> > +# now we need to check that after the year 2242, e2fsck does not
> > +# modify dates with extra_xtime=3
> > +
> > +# restore the unrepaired filesystem
> > +mv $TMPFILE.backup $TMPFILE
> > +
> > +#retry the repair
> > +E2FSCK_TIME=9270393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1
> > +
> > +# check that the 1909 file is unaltered (i.e. it has a post-2378 date)
> > +get_file_xtime_and_extra year-1909
> > +
> > +cmp -s $TIMESTAMPS $EXP
> > +status=$?
> > +
> > +if [ "$status" = 0 ] ; then
> > +    echo "$test_name: $test_description: ok"
> > +    touch $test_name.ok
> > +else
> > +    echo "$test_name: $test_description: failed"
> > +    diff $DIFF_OPTS $EXP $TIMESTAMPS > $test_name.failed
> > +fi
> > +
> > +unset OUT TIMESTAMPS EXP FSCK_OPT
> > +
> > +else #if test -x $DEBUGFS_EXE; then
> > +    echo "$test_name: $test_description: skipped"
> > +fi
> > +
> > -- 
> > 1.8.1.2
> > 
> > 
> > 
> --
> 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