[01/12] Ext4: Fix extended timestamp encoding and decoding
diff mbox

Message ID 20151124193646.GA3482@thunk.org
State New
Headers show

Commit Message

Theodore Y. Ts'o Nov. 24, 2015, 7:36 p.m. UTC
This is the patch I would prefer to use (and in fact which I have
added to the ext4 tree):

There are issues with 32-bit vs 64-bit encoding of times before
January 1, 1970, which are handled with this patch which is not
handled with what you have in your patch series.  So I'd prefer if you
drop this patch, and I'll get this sent to Linus as a bug fix for 4.4.

Cheers,

					- Ted


commit e0d738b05d484487b7e1e3c6d537da8bbef80c86
Author: David Turner <novalis@novalis.org>
Date:   Tue Nov 24 14:34:37 2015 -0500

    ext4: Fix handling of extended tv_sec
    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>
    Signed-off-by: Theodore Ts'o <tytso@mit.edu>
    Reported-by: Mark Harris <mh8928@yahoo.com>
    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
    Cc: stable@vger.kernel.org

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

Comments

Arnd Bergmann Nov. 24, 2015, 8:10 p.m. UTC | #1
On Tuesday 24 November 2015 14:36:46 Theodore Ts'o wrote:
> This is the patch I would prefer to use (and in fact which I have
> added to the ext4 tree):
> 
> There are issues with 32-bit vs 64-bit encoding of times before
> January 1, 1970, which are handled with this patch which is not
> handled with what you have in your patch series.  So I'd prefer if you
> drop this patch, and I'll get this sent to Linus as a bug fix for 4.4.

I'm happy with either one. Apparently both Davids have arrived with
almost the same algorithm and implementation, with the exception of
the pre-1970 handling you mention there.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Howells Nov. 26, 2015, 3:28 p.m. UTC | #2
Theodore Ts'o <tytso@mit.edu> wrote:

> This is the patch I would prefer to use (and in fact which I have
> added to the ext4 tree):
> 
> There are issues with 32-bit vs 64-bit encoding of times before
> January 1, 1970, which are handled with this patch which is not
> handled with what you have in your patch series.  So I'd prefer if you
> drop this patch, and I'll get this sent to Linus as a bug fix for 4.4.

Fine by me.

Acked-by: David Howells <dhowells@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Y. Ts'o Nov. 29, 2015, 2:45 a.m. UTC | #3
On Tue, Nov 24, 2015 at 09:10:53PM +0100, Arnd Bergmann wrote:
> On Tuesday 24 November 2015 14:36:46 Theodore Ts'o wrote:
> > This is the patch I would prefer to use (and in fact which I have
> > added to the ext4 tree):
> > 
> > There are issues with 32-bit vs 64-bit encoding of times before
> > January 1, 1970, which are handled with this patch which is not
> > handled with what you have in your patch series.  So I'd prefer if you
> > drop this patch, and I'll get this sent to Linus as a bug fix for 4.4.
> 
> I'm happy with either one. Apparently both Davids have arrived with
> almost the same algorithm and implementation, with the exception of
> the pre-1970 handling you mention there.

I was doing some testing on x86, which leads me to ask --- what's the
current thinking about post y2038 on 32-bit platforms such as x86?  I
see that there was some talk about using struct timespec64, but we
haven't made the transition in the VFS interfaces yet, despite a
comment in an LWN article from 2014 stating that "the first steps have
been taken; hopefully the rest will follow before too long".

Cheers,

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Nov. 29, 2015, 9:30 p.m. UTC | #4
On Saturday 28 November 2015 21:45:55 Theodore Ts'o wrote:
> On Tue, Nov 24, 2015 at 09:10:53PM +0100, Arnd Bergmann wrote:
> > On Tuesday 24 November 2015 14:36:46 Theodore Ts'o wrote:
> > > This is the patch I would prefer to use (and in fact which I have
> > > added to the ext4 tree):
> > > 
> > > There are issues with 32-bit vs 64-bit encoding of times before
> > > January 1, 1970, which are handled with this patch which is not
> > > handled with what you have in your patch series.  So I'd prefer if you
> > > drop this patch, and I'll get this sent to Linus as a bug fix for 4.4.
> > 
> > I'm happy with either one. Apparently both Davids have arrived with
> > almost the same algorithm and implementation, with the exception of
> > the pre-1970 handling you mention there.
> 
> I was doing some testing on x86, which leads me to ask --- what's the
> current thinking about post y2038 on 32-bit platforms such as x86?  I
> see that there was some talk about using struct timespec64, but we
> haven't made the transition in the VFS interfaces yet, despite a
> comment in an LWN article from 2014 stating that "the first steps have
> been taken; hopefully the rest will follow before too long".

The approach in my initial VFS series was to introduce 'struct inode_time',
but I have basically abandoned that idea now, after we decided to introduce
'timespec64' inside of the kernel and use that for other subsystems.
The rought plan is now to have separate time64_t and u32 seconds/nanoseconds
values in 'struct inode', 'struct iattr' and 'struct kstat' and use
inline functions or macros to extract or set them as time64_t or timespec64
in file system code, but that code is not written yet.

I'm mostly coordinating the y2038 work at the moment, but that means that
a lot of the work is going into individual drivers that a single person
can easily handle. We've had a couple of people who tried looking at VFS,
but none of them followed through, so it got delayed a bit. However,
Deepa Dinamani is now looking y2038 for VFS and individual file systems
as part of her Outreachy internship and I'm optimistic that we'll soon
be making progress again here with her work.

The other large missing piece is the system call implementation. I have
posted a series earlier this year before my parental leave, and it's
currently lacking review from libc folks, and blocked on me to update
the series and post it again.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Y. Ts'o Nov. 30, 2015, 2:16 p.m. UTC | #5
On Sun, Nov 29, 2015 at 10:30:39PM +0100, Arnd Bergmann wrote:
> The other large missing piece is the system call implementation. I have
> posted a series earlier this year before my parental leave, and it's
> currently lacking review from libc folks, and blocked on me to update
> the series and post it again.

I assume that this also means there hasn't been much thought about
userspace support above libc?  i.e., how to take a 64-bit time64_t (or
changing the size of time_t) and translating that to a string using
some kind of version of ctime() and asctime(), and how to parse a
post-2038 date string and turning it into a 64-bit time_t on a 32-bit
platform?

The reason why I'm asking is because I'm thinking about how to add the
appropriate regression test support to e2fsprogs for 32-bit platforms.
I'm probably going to just skip the tests on architectures where
sizeof(time_t) == 4 for now, since with a 32-bit time_t adding support
for post-2038 in a e2fsprogs-specific way is (a) something I don't
have time for, and (b) probably a waste of time since presumably we
will either need to have a more general solution, or simply decide to
give up on 32-bit platforms by 2038....

Cheers,

   	       	     	    	      	 - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Nov. 30, 2015, 2:37 p.m. UTC | #6
On Monday 30 November 2015 09:16:05 Theodore Ts'o wrote:
> On Sun, Nov 29, 2015 at 10:30:39PM +0100, Arnd Bergmann wrote:
> > The other large missing piece is the system call implementation. I have
> > posted a series earlier this year before my parental leave, and it's
> > currently lacking review from libc folks, and blocked on me to update
> > the series and post it again.
> 
> I assume that this also means there hasn't been much thought about
> userspace support above libc?  i.e., how to take a 64-bit time64_t (or
> changing the size of time_t) and translating that to a string using
> some kind of version of ctime() and asctime(), and how to parse a
> post-2038 date string and turning it into a 64-bit time_t on a 32-bit
> platform?
> 
> The reason why I'm asking is because I'm thinking about how to add the
> appropriate regression test support to e2fsprogs for 32-bit platforms.
> I'm probably going to just skip the tests on architectures where
> sizeof(time_t) == 4 for now, since with a 32-bit time_t adding support
> for post-2038 in a e2fsprogs-specific way is (a) something I don't
> have time for, and (b) probably a waste of time since presumably we
> will either need to have a more general solution, or simply decide to
> give up on 32-bit platforms by 2038....

We are definitely going to be using 32-bit embedded platforms in 2038,
but we won't be using a 32-bit time_t then, so basing the check on
sizeof(time_t) sounds reasonable. I assume most generic distros will
stay with 32-bit time_t for compatibility reasons and just not give
long term support for 32-bit architectures, while the embedded
distros will move over to 64-bit time_t, but on those you recompile
all user space for each product anyway.

The glibc functions should all work with a 64-bit time_t as they do
today on 64-bit architectures. There is an open discussion on how
you move to 64-bit time_t. With the current
glibc plan at https://sourceware.org/glibc/wiki/Y2038ProofnessDesign,
you will have to set -D_TIME_BITS=64 to enable it explicitly, but
I'd also like to see a way to build a glibc that defaults to that
and does not allow backwards compatibility, which is important for
folks that want to ship a system that has they can guarantee to
survive 2038.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Elmar Stellnberger Nov. 30, 2015, 2:46 p.m. UTC | #7
On 30.11.2015 15:16, Theodore Ts'o wrote:
> On Sun, Nov 29, 2015 at 10:30:39PM +0100, Arnd Bergmann wrote:
>> The other large missing piece is the system call implementation. I have
>> posted a series earlier this year before my parental leave, and it's
>> currently lacking review from libc folks, and blocked on me to update
>> the series and post it again.
>
> I assume that this also means there hasn't been much thought about
> userspace support above libc?  i.e., how to take a 64-bit time64_t (or
> changing the size of time_t) and translating that to a string using
> some kind of version of ctime() and asctime(), and how to parse a
> post-2038 date string and turning it into a 64-bit time_t on a 32-bit
> platform?
>

   Arnd, I would just like to tell you how much I welcome your decision 
for a new __kernel_time64_t!
   As a time[64]_t is basically well defined counting artificial seconds 
since the epoch (1970-01-01 00:00) where every year divisible by four is 
a leap year that is for the meanwhile already sufficient to make use of 
your new type. I just think about the Mayan calendar application which I 
have implemented last year (Though I have not brought it to a 
publishable state yet). A single typedef should be sufficient to let it 
make use of time64_t (it directly uses this type as well as long long 
internally for its calculations rather than the glibc time format 
functions).

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

Patch
diff mbox

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 750063f..fddce29 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>
@@ -727,19 +728,53 @@  struct move_extent {
 	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
 	    (einode)->i_extra_isize))			\
 
+/*
+ * We use 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)			       \