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 <[email protected]>
> Reported-by: Mark Harris <[email protected]>
> 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 <[email protected]>
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 <[email protected]>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
