[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <38140D27-E451-4BF1-9445-D319652E1BCF@dilger.ca>
Date: Mon, 31 Mar 2014 09:34:52 -0600
From: Andreas Dilger <adilger@...ger.ca>
To: Conrad Meyer <cemeyer@...edu>
Cc: Theodore Ts'o <tytso@....edu>,
Andreas Dilger <adilger.kernel@...ger.ca>,
"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Conrad Meyer <cse.cem@...il.com>
Subject: Re: [PATCH] fs: ext4: Sign-extend tv_sec after ORing in epoch bits
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@...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@...il.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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists