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]
Message-ID: <CAK8P3a2NXEC_+VNw4bkorisUCfVg_94ooTNN+SZe78dF_uqmWQ@mail.gmail.com>
Date:   Tue, 24 Jul 2018 17:28:36 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Ernesto A. Fernández 
        <ernesto.mnd.fernandez@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        y2038 Mailman List <y2038@...ts.linaro.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        "# 3.4.x" <stable@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vyacheslav Dubeyko <slava@...eyko.com>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior

On Thu, Jul 12, 2018 at 12:46 AM, Ernesto A. Fernández
<ernesto.mnd.fernandez@...il.com> wrote:

>> -#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)
>> +static inline time64_t __hfs_m_to_utime(__be32 mt)
>> +{
>> +     time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
>> +
>> +     /*
>> +      * Times past 2040-02-06 06:28 are assumed to be invalid,
>> +      * matching the MacOS behavior.
>> +      */
>> +     if (ut > 2082844800U + UINT_MAX)
>
> I'm not sure what you were going for here, but this isn't right. Times
> as early as 2036 will be considered invalid.

Right, the calculation makes no sense, I have no idea how I ever
got there. I was trying to check for the 2040 date, and rearranged my
code multiple times for that. What I probably meant was

     if (ut + 2082844800U > UINT_MAX)

or

    if (ut > UINT_MAX - 2082844800U)

>> +             ut = 0;
>> +
>> +     return ut + sys_tz.tz_minuteswest * 60;
>> +}
>>
>> +static inline __be32 __hfs_u_to_mtime(time64_t ut)
>> +{
>> +     ut -= - sys_tz.tz_minuteswest * 60;
>            ^^^^^
>         An extra minus.

I saw that Andrew had added a fix on top and assumed he got it right, but
upon checking again later I see that it's still there.

>> +
>> +     /*
>> +      * MacOS wraps "invalid" times after 2040 when writing back, so
>> +      * let's do the same here.
>> +      */
>> +     return cpu_to_be32(lower_32_bits(ut + 2082844800U));
>> +}
>>  #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)
>>
>> -#define hfs_m_to_utime(time) (struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
>> -#define hfs_u_to_mtime(time) __hfs_u_to_mtime((time).tv_sec)
>> +#define hfs_m_to_utime(time)   (struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
>> +#define hfs_u_to_mtime(time)   __hfs_u_to_mtime((time).tv_sec)
>
> Are the new spaces intentional?

No, I should drop that.

>>  #define hfs_mtime()          __hfs_u_to_mtime(get_seconds())
>>
>>  static inline const char *hfs_mdb_name(struct super_block *sb)
>> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
>> index d9255abafb81..7f0943e540a0 100644
>> --- a/fs/hfsplus/hfsplus_fs.h
>> +++ b/fs/hfsplus/hfsplus_fs.h
>> @@ -530,9 +530,29 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
>>                      void **data, int op, int op_flags);
>>  int hfsplus_read_wrapper(struct super_block *sb);
>>
>> -/* time macros */
>> -#define __hfsp_mt2ut(t)              (be32_to_cpu(t) - 2082844800U)
>> -#define __hfsp_ut2mt(t)              (cpu_to_be32(t + 2082844800U))
>> +/* time helpers */
>> +static inline time64_t __hfsp_mt2ut(__be32 mt)
>> +{
>> +     time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
>> +
>> +     /*
>> +      * Times past 2040-02-06 06:28 are assumed to be invalid,
>> +      * matching the MacOS behavior.
>> +      */
>> +     if (ut > 2082844800U + UINT_MAX)
>
> Same as before, 2036-2040 will be invalid.
>
>
> For the record, your original solution (supporting the 1970-2106 range)
> still makes more sense to me. It seems Apple is not using the 1904-1970
> range for anything; if they are still supporting hfsplus by the 2030s I
> assume they will deal with this in a similar way.

I'm definitely fine with it either way. The current approach was based on
the feedback I got from Viacheslav Dubeyko, and on what I found
in the various other implementations. Viacheslav, are you ok with
changing the hfs code back to my original interpretation of the
timestamps, using the 1970..2106 range?

Andrew, can you drop both my hfs/hfsplus patches for now? At
this point it seems better to start from scratch than to fix up the
bugs individually.

      Arnd

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ