linux-kernel - Re: [Y2038] [PATCH 13/16] hfs/hfsplus: use 64-bit inode timestamps

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a1Bvdb4Xn1Aqfnxnn24HPb6FXxAAVwq8ypO31AqoR1hBw@mail.gmail.com>
Date:   Wed, 13 Nov 2019 09:06:57 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Viacheslav Dubeyko <slava@...eyko.com>
Cc:     y2038 Mailman List <y2038@...ts.linaro.org>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        Ernesto A. Fernández 
        <ernesto.mnd.fernandez@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [Y2038] [PATCH 13/16] hfs/hfsplus: use 64-bit inode timestamps

On Wed, Nov 13, 2019 at 7:00 AM Viacheslav Dubeyko <slava@...eyko.com> wrote:
> > On Nov 9, 2019, at 12:32 AM, Arnd Bergmann <arnd@...db.de> wrote:
> >  * There are two time systems.  Both are based on seconds since
> >  * a particular time/date.
> > - *   Unix:   unsigned lil-endian since 00:00 GMT, Jan. 1, 1970
> > + *   Unix:   signed little-endian since 00:00 GMT, Jan. 1, 1970
> >  *    mac:    unsigned big-endian since 00:00 GMT, Jan. 1, 1904
> >  *
> > + * HFS implementations are highly inconsistent, this one matches the
> > + * traditional behavior of 64-bit Linux, giving the most useful
> > + * time range between 1970 and 2106, by treating any on-disk timestamp
> > + * under 2082844800U (Jan 1 1970) as a time between 2040 and 2106.
> >  */
> > -#define __hfs_u_to_mtime(sec)        cpu_to_be32(sec + 2082844800U - sys_tz.tz_minuteswest * 60)
> > -#define __hfs_m_to_utime(sec)        (be32_to_cpu(sec) - 2082844800U  + sys_tz.tz_minuteswest * 60)
>
> I believe it makes sense to introduce some constant instead of hardcoded value (2082844800U and 60).
> It will be easier to understand the code without necessity to take a look into the comments.
> What do you think?

Every other user of sys_tz.tz_minuteswest uses a plain '60', I think that one
is easy enough to understand from context. Naming the other constant
is a good idea, I've now folded the change below into my patch.

Thanks for the review!

      Arnd

8<-----
diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index 26733051ee50..f71c384064c8 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -247,22 +247,24 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb);
  *
  * HFS implementations are highly inconsistent, this one matches the
  * traditional behavior of 64-bit Linux, giving the most useful
  * time range between 1970 and 2106, by treating any on-disk timestamp
- * under 2082844800U (Jan 1 1970) as a time between 2040 and 2106.
+ * under HFS_UTC_OFFSET (Jan 1 1970) as a time between 2040 and 2106.
  */
+#define HFS_UTC_OFFSET 2082844800U
+
 static inline time64_t __hfs_m_to_utime(__be32 mt)
 {
-       time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
+       time64_t ut = (u32)(be32_to_cpu(mt) - HFS_UTC_OFFSET);

        return ut + sys_tz.tz_minuteswest * 60;
 }

 static inline __be32 __hfs_u_to_mtime(time64_t ut)
 {
        ut -= sys_tz.tz_minuteswest * 60;

-       return cpu_to_be32(lower_32_bits(ut) + 2082844800U);
+       return cpu_to_be32(lower_32_bits(ut) + HFS_UTC_OFFSET);
 }
 #define HFS_I(inode)   (container_of(inode, struct hfs_inode_info, vfs_inode))
 #define HFS_SB(sb)     ((struct hfs_sb_info *)(sb)->s_fs_info)

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 22d0a22c41a3..3b03fff68543 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -538,20 +538,22 @@ int hfsplus_read_wrapper(struct super_block *sb);
  *
  * HFS+ implementations are highly inconsistent, this one matches the
  * traditional behavior of 64-bit Linux, giving the most useful
  * time range between 1970 and 2106, by treating any on-disk timestamp
- * under 2082844800U (Jan 1 1970) as a time between 2040 and 2106.
+ * under HFSPLUS_UTC_OFFSET (Jan 1 1970) as a time between 2040 and 2106.
  */
+#define HFSPLUS_UTC_OFFSET 2082844800U
+
 static inline time64_t __hfsp_mt2ut(__be32 mt)
 {
-       time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
+       time64_t ut = (u32)(be32_to_cpu(mt) - HFSPLUS_UTC_OFFSET);

        return ut;
 }

 static inline __be32 __hfsp_ut2mt(time64_t ut)
 {
-       return cpu_to_be32(lower_32_bits(ut) + 2082844800U);
+       return cpu_to_be32(lower_32_bits(ut) + HFSPLUS_UTC_OFFSET);
 }

 /* compatibility */
 #define hfsp_mt2ut(t)          (struct timespec64){ .tv_sec = __hfsp_mt2ut(t) }