lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Mon, 18 Jun 2007 04:49:14 -0600
From:	Andreas Dilger <adilger@...sterfs.com>
To:	Kalpak Shah <kalpak@...sterfs.com>
Cc:	linux-ext4 <linux-ext4@...r.kernel.org>,
	Mingming Cao <cmm@...ibm.com>, Andrew Morton <akpm@...l.org>
Subject: Re: Correction to nanosecond timestamp patch for 64-bit arch

On Jun 18, 2007  15:39 +0530, Kalpak Shah wrote:
> On Sun, 2007-06-17 at 18:32 +0530, Kalpak Shah wrote:
> > Index: linux-2.6.21/include/linux/ext4_fs.h
> > ===================================================================
> > --- linux-2.6.21.orig/include/linux/ext4_fs.h
> > +++ linux-2.6.21/include/linux/ext4_fs.h
> > @@ -366,9 +366,9 @@ static inline __le32 ext4_encode_extra_t
> >  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) >> 2;
> > +	       time->tv_sec |= (__u64)((signed)le32_to_cpu(extra) &
> > +				       EXT4_EPOCH_MASK) << 32;
> > +       time->tv_nsec = ((signed)le32_to_cpu(extra) & EXT4_NSEC_MASK) >> 2;
> >  }
> >  
> 
> I am not too sure about the above hunk. tv_sec would not need any
> (signed) cast since it is being ORed. And nsec should always lie between
> 0 and 1e9.
> 
> But the following patch is definitely correct and needs to be applied.

Hmm, this makes me think that the shifting for the "epoch" needs to be 
done with 31 bits instead of 32, otherwise the time between 0x7fffffff
and 0xffffffff will always appear to be negative.

I'm beginning to wonder at the value of trying to save any times with
negative timestamp values.  That might have seemed useful in 1970 when
it was only a few weeks or months in the past, but it seems like a waste
of bits today.  It would seem prudent to just truncate times on disk to
0 if the timestamp is negative?  The original bug report was related to
setting the time to Jan 1 1970 in some timezone that causes this to be
a negative value, so limiting the timestamp to 0 in such cases wouldn't
be considered a limitation.

According to the original bug report, storing negative times isn't even
possible to do with some tools, and might be considered a bug in "date"
as much as anything.  If the problem is the inconsistency of times between
32-bit and 64-bit systems this could be resolved by always treating the
on-disk value as an unsigned integer, and it would be capped at +2^31
(2038) on 32-bit systems for the next couple of years until "date" is fixed.

>  #define EXT4_INODE_GET_XTIME(xtime, inode, raw_inode)                         \
>  do {                                                                          \
> -       (inode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);               \
> +       (inode)->xtime.tv_sec = (signed)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);            \
> @@ -399,7 +399,8 @@ do {                                                                               \
>  #define EXT4_EINODE_GET_XTIME(xtime, einode, raw_inode)                               \
>  do {                                                                          \
>         if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime))                      \
> -               (einode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);      \
> +               (einode)->xtime.tv_sec =                                       \
> +                               (signed)le32_to_cpu((raw_inode)->xtime);       \
>         if (EXT4_FITS_IN_INODE(raw_inode, einode, xtime ## _extra))            \
>                 ext4_decode_extra_time(&(einode)->xtime,                       \
>                                        raw_inode->xtime ## _extra);            \

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ