* [PATCH] fs: ext4: Sign-extend tv_sec after ORing in epoch bits
@ 2014-03-30 14:58 Conrad Meyer
2014-03-31 15:34 ` Andreas Dilger
0 siblings, 1 reply; 4+ messages in thread
From: Conrad Meyer @ 2014-03-30 14:58 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger; +Cc: linux-ext4, linux-kernel, Conrad Meyer
Fixes kernel.org bug #23732.
Background: ext4 stores time as a 34-bit quantity; 2 bits in some extra
bits unneeded for nanoseconds in the inode, and 32 bits in the seconds
field.
On systems with 64-bit time_t, the EXT4_*INODE_GET_XTIME() code
incorrectly sign-extended the low 32-bits of the seconds quantity before
ORing in the 2 "epoch" bits from nanoseconds.
This patch ORs in the 2 higher bits, then sign extends the 34-bit signed
number to 64 bits.
Signed-off-by: Conrad Meyer <cse.cem@gmail.com>
---
Patch against next-20140328.
Note, the on-disk format has always been written correctly. It was just
interpreted incorrectly.
Repro:
Before:
$ touch -d 2038-01-31 test-123
$ sudo sh -c "echo 3 > /proc/sys/vm/drop_caches"
$ ls -ld test-123
drwxrwxr-x 2 cmeyer cmeyer 4096 Dec 25 1901 test-123
After:
$ ls -ld test-123
drwxrwxr-x 2 cmeyer cmeyer 4096 Jan 31 2038 test-123
Thanks!
---
fs/ext4/ext4.h | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index f4f889e..07ee03d 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -710,6 +710,8 @@ struct move_extent {
#define EXT4_EPOCH_BITS 2
#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
#define EXT4_NSEC_MASK (~0UL << EXT4_EPOCH_BITS)
+#define EXT4_EPOCH_SIGN (((1UL << (EXT4_EPOCH_BITS - 1)) << 16) << 16)
+#define EXT4_SIGN_EXT (~((((1UL << EXT4_EPOCH_BITS) << 16) << 16) - 1))
/*
* Extended fields will fit into an inode if the filesystem was formatted
@@ -761,19 +763,23 @@ do { \
#define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode) \
do { \
- (inode)->xtime.tv_sec = (signed)le32_to_cpu((raw_inode)->xtime); \
+ (inode)->xtime.tv_sec = (__u64)le32_to_cpu((raw_inode)->xtime); \
if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra)) \
ext4_decode_extra_time(&(inode)->xtime, \
raw_inode->xtime ## _extra); \
else \
(inode)->xtime.tv_nsec = 0; \
+ if (sizeof((inode)->xtime.tv_sec) > 4) { \
+ if ((inode)->xtime.tv_sec & EXT4_EPOCH_SIGN) \
+ (inode)->xtime.tv_sec |= EXT4_SIGN_EXT; \
+ } \
} while (0)
#define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode) \
do { \
if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime)) \
(einode)->xtime.tv_sec = \
- (signed)le32_to_cpu((raw_inode)->xtime); \
+ (__u64)le32_to_cpu((raw_inode)->xtime); \
else \
(einode)->xtime.tv_sec = 0; \
if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra)) \
@@ -781,6 +787,10 @@ do { \
raw_inode->xtime ## _extra); \
else \
(einode)->xtime.tv_nsec = 0; \
+ if (sizeof((einode)->xtime.tv_sec) > 4) { \
+ if ((einode)->xtime.tv_sec & EXT4_EPOCH_SIGN) \
+ (einode)->xtime.tv_sec |= EXT4_SIGN_EXT; \
+ } \
} while (0)
#define i_disk_version osd1.linux1.l_i_version
--
1.9.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: ext4: Sign-extend tv_sec after ORing in epoch bits
2014-03-30 14:58 [PATCH] fs: ext4: Sign-extend tv_sec after ORing in epoch bits Conrad Meyer
@ 2014-03-31 15:34 ` Andreas Dilger
2014-03-31 15:42 ` Conrad Meyer
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Dilger @ 2014-03-31 15:34 UTC (permalink / raw)
To: Conrad Meyer
Cc: Theodore Ts'o, Andreas Dilger, linux-ext4, linux-kernel,
Conrad Meyer
Hmm,
I thought there was a separate patch to fix this a few months ago, that was "more correct" than this one? Did that not land?
Cheers, Andreas
> On Mar 30, 2014, at 8:58, Conrad Meyer <cemeyer@uw.edu> wrote:
>
> Fixes kernel.org bug #23732.
>
> Background: ext4 stores time as a 34-bit quantity; 2 bits in some extra
> bits unneeded for nanoseconds in the inode, and 32 bits in the seconds
> field.
>
> On systems with 64-bit time_t, the EXT4_*INODE_GET_XTIME() code
> incorrectly sign-extended the low 32-bits of the seconds quantity before
> ORing in the 2 "epoch" bits from nanoseconds.
>
> This patch ORs in the 2 higher bits, then sign extends the 34-bit signed
> number to 64 bits.
>
> Signed-off-by: Conrad Meyer <cse.cem@gmail.com>
> ---
> Patch against next-20140328.
>
> Note, the on-disk format has always been written correctly. It was just
> interpreted incorrectly.
>
> Repro:
> Before:
> $ touch -d 2038-01-31 test-123
> $ sudo sh -c "echo 3 > /proc/sys/vm/drop_caches"
> $ ls -ld test-123
> drwxrwxr-x 2 cmeyer cmeyer 4096 Dec 25 1901 test-123
>
> After:
> $ ls -ld test-123
> drwxrwxr-x 2 cmeyer cmeyer 4096 Jan 31 2038 test-123
>
> Thanks!
> ---
> fs/ext4/ext4.h | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index f4f889e..07ee03d 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -710,6 +710,8 @@ struct move_extent {
> #define EXT4_EPOCH_BITS 2
> #define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
> #define EXT4_NSEC_MASK (~0UL << EXT4_EPOCH_BITS)
> +#define EXT4_EPOCH_SIGN (((1UL << (EXT4_EPOCH_BITS - 1)) << 16) << 16)
> +#define EXT4_SIGN_EXT (~((((1UL << EXT4_EPOCH_BITS) << 16) << 16) - 1))
>
> /*
> * Extended fields will fit into an inode if the filesystem was formatted
> @@ -761,19 +763,23 @@ do { \
>
> #define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode) \
> do { \
> - (inode)->xtime.tv_sec = (signed)le32_to_cpu((raw_inode)->xtime); \
> + (inode)->xtime.tv_sec = (__u64)le32_to_cpu((raw_inode)->xtime); \
> if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra)) \
> ext4_decode_extra_time(&(inode)->xtime, \
> raw_inode->xtime ## _extra); \
> else \
> (inode)->xtime.tv_nsec = 0; \
> + if (sizeof((inode)->xtime.tv_sec) > 4) { \
> + if ((inode)->xtime.tv_sec & EXT4_EPOCH_SIGN) \
> + (inode)->xtime.tv_sec |= EXT4_SIGN_EXT; \
> + } \
> } while (0)
>
> #define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode) \
> do { \
> if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime)) \
> (einode)->xtime.tv_sec = \
> - (signed)le32_to_cpu((raw_inode)->xtime); \
> + (__u64)le32_to_cpu((raw_inode)->xtime); \
> else \
> (einode)->xtime.tv_sec = 0; \
> if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra)) \
> @@ -781,6 +787,10 @@ do { \
> raw_inode->xtime ## _extra); \
> else \
> (einode)->xtime.tv_nsec = 0; \
> + if (sizeof((einode)->xtime.tv_sec) > 4) { \
> + if ((einode)->xtime.tv_sec & EXT4_EPOCH_SIGN) \
> + (einode)->xtime.tv_sec |= EXT4_SIGN_EXT; \
> + } \
> } while (0)
>
> #define i_disk_version osd1.linux1.l_i_version
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: ext4: Sign-extend tv_sec after ORing in epoch bits
2014-03-31 15:34 ` Andreas Dilger
@ 2014-03-31 15:42 ` Conrad Meyer
2014-04-01 2:56 ` Theodore Ts'o
0 siblings, 1 reply; 4+ messages in thread
From: Conrad Meyer @ 2014-03-31 15:42 UTC (permalink / raw)
To: Andreas Dilger
Cc: Conrad Meyer, Theodore Ts'o, Andreas Dilger, linux-ext4,
linux-kernel
The problem exists in mainline (v3.14) and linux-next (20140328), so
it looks like it didn't land. Unless it's queued in an ext4 tree and
didn't get selected for Linus for some reason?
Thanks,
Conrad
On Mon, Mar 31, 2014 at 8:34 AM, Andreas Dilger <adilger@dilger.ca> wrote:
> Hmm,
> I thought there was a separate patch to fix this a few months ago, that was "more correct" than this one? Did that not land?
>
> Cheers, Andreas
>
>> On Mar 30, 2014, at 8:58, Conrad Meyer <cemeyer@uw.edu> wrote:
>>
>> Fixes kernel.org bug #23732.
>>
>> Background: ext4 stores time as a 34-bit quantity; 2 bits in some extra
>> bits unneeded for nanoseconds in the inode, and 32 bits in the seconds
>> field.
>>
>> On systems with 64-bit time_t, the EXT4_*INODE_GET_XTIME() code
>> incorrectly sign-extended the low 32-bits of the seconds quantity before
>> ORing in the 2 "epoch" bits from nanoseconds.
>>
>> This patch ORs in the 2 higher bits, then sign extends the 34-bit signed
>> number to 64 bits.
>>
>> Signed-off-by: Conrad Meyer <cse.cem@gmail.com>
>> ---
>> Patch against next-20140328.
>>
>> Note, the on-disk format has always been written correctly. It was just
>> interpreted incorrectly.
>>
>> Repro:
>> Before:
>> $ touch -d 2038-01-31 test-123
>> $ sudo sh -c "echo 3 > /proc/sys/vm/drop_caches"
>> $ ls -ld test-123
>> drwxrwxr-x 2 cmeyer cmeyer 4096 Dec 25 1901 test-123
>>
>> After:
>> $ ls -ld test-123
>> drwxrwxr-x 2 cmeyer cmeyer 4096 Jan 31 2038 test-123
>>
>> Thanks!
>> ---
>> fs/ext4/ext4.h | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index f4f889e..07ee03d 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -710,6 +710,8 @@ struct move_extent {
>> #define EXT4_EPOCH_BITS 2
>> #define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1)
>> #define EXT4_NSEC_MASK (~0UL << EXT4_EPOCH_BITS)
>> +#define EXT4_EPOCH_SIGN (((1UL << (EXT4_EPOCH_BITS - 1)) << 16) << 16)
>> +#define EXT4_SIGN_EXT (~((((1UL << EXT4_EPOCH_BITS) << 16) << 16) - 1))
>>
>> /*
>> * Extended fields will fit into an inode if the filesystem was formatted
>> @@ -761,19 +763,23 @@ do { \
>>
>> #define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode) \
>> do { \
>> - (inode)->xtime.tv_sec = (signed)le32_to_cpu((raw_inode)->xtime); \
>> + (inode)->xtime.tv_sec = (__u64)le32_to_cpu((raw_inode)->xtime); \
>> if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra)) \
>> ext4_decode_extra_time(&(inode)->xtime, \
>> raw_inode->xtime ## _extra); \
>> else \
>> (inode)->xtime.tv_nsec = 0; \
>> + if (sizeof((inode)->xtime.tv_sec) > 4) { \
>> + if ((inode)->xtime.tv_sec & EXT4_EPOCH_SIGN) \
>> + (inode)->xtime.tv_sec |= EXT4_SIGN_EXT; \
>> + } \
>> } while (0)
>>
>> #define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode) \
>> do { \
>> if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime)) \
>> (einode)->xtime.tv_sec = \
>> - (signed)le32_to_cpu((raw_inode)->xtime); \
>> + (__u64)le32_to_cpu((raw_inode)->xtime); \
>> else \
>> (einode)->xtime.tv_sec = 0; \
>> if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra)) \
>> @@ -781,6 +787,10 @@ do { \
>> raw_inode->xtime ## _extra); \
>> else \
>> (einode)->xtime.tv_nsec = 0; \
>> + if (sizeof((einode)->xtime.tv_sec) > 4) { \
>> + if ((einode)->xtime.tv_sec & EXT4_EPOCH_SIGN) \
>> + (einode)->xtime.tv_sec |= EXT4_SIGN_EXT; \
>> + } \
>> } while (0)
>>
>> #define i_disk_version osd1.linux1.l_i_version
>> --
>> 1.9.0
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: ext4: Sign-extend tv_sec after ORing in epoch bits
2014-03-31 15:42 ` Conrad Meyer
@ 2014-04-01 2:56 ` Theodore Ts'o
0 siblings, 0 replies; 4+ messages in thread
From: Theodore Ts'o @ 2014-04-01 2:56 UTC (permalink / raw)
To: Conrad Meyer
Cc: Andreas Dilger, Conrad Meyer, Andreas Dilger, linux-ext4, linux-kernel
On Mon, Mar 31, 2014 at 08:42:06AM -0700, Conrad Meyer wrote:
> The problem exists in mainline (v3.14) and linux-next (20140328), so
> it looks like it didn't land. Unless it's queued in an ext4 tree and
> didn't get selected for Linus for some reason?
There were some proposals for a different encoding that would be
better, that would have required some e2fsprogs changes. Since this
is a long range problem (that doesn't hit until 2038) it's not been
high priority to deal with, but I haven't forgotten it. I've just had
higher priority items on my todo list.
The huge long thread can be found here:
http://thread.gmane.org/gmane.comp.file-systems.ext4/40978
What this is blocked on is I wanted to have some better ways of
setting times using the old and the proposed new encoding convention,
so we could have proper regression tests for the changes in e2fsck.
That way we can make sure the right thing really happens with 32-bit
kernels, 64-bit kernels, 32-bit e2fsprogs, 64-bit e2fsprogs, etc.,
with file systems using both the old and the newer encoding.
(Yes, I'm paranoid that way. Regression tests are _important_.)
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-04-01 2:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-30 14:58 [PATCH] fs: ext4: Sign-extend tv_sec after ORing in epoch bits Conrad Meyer
2014-03-31 15:34 ` Andreas Dilger
2014-03-31 15:42 ` Conrad Meyer
2014-04-01 2:56 ` Theodore Ts'o
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.