linux-ext4 - Re: Y2038 bug in ext4 recently

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: <CAK8P3a3_2u0KpagBcc0mG6fxCnftqaMyXgYhL2z8GoBWqfSrhA@mail.gmail.com>
Date:   Tue, 22 Aug 2017 21:35:46 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     Deepa Dinamani <deepa.kernel@...il.com>,
        "Theodore Ts'o" <tytso@....edu>, Wang Shilong <wshilong@....com>,
        Wang Shilong <wangshilong1991@...il.com>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
        Shuichi Ihara <sihara@....com>, Li Xi <lixi@....com>,
        Jan Kara <jack@...e.cz>
Subject: Re: Y2038 bug in ext4 recently_deleted() function

On Tue, Aug 22, 2017 at 6:20 PM, Andreas Dilger <adilger@...ger.ca> wrote:
> On Aug 22, 2017, at 9:18 AM, Arnd Bergmann <arnd@...db.de> wrote:
>>
>> On Fri, Aug 18, 2017 at 6:09 PM, Andreas Dilger <adilger@...ger.ca> wrote:
>>>
>>>>>> So moving to the case of a 32 bit machine:
>>>>>>
>>>>>> get_seconds() can return values until year 2106. And, recentcy at max
>>>>>> can only be 35. Analyzing the current line:
>>>>>>
>>>>>> if (dtime && (dtime < now) && (now < dtime + recentcy))
>>>>>>
>>>>>> The above equation should work fine at least until 35 seconds before
>>>>>> y2038 deadline.
>>>>>
>>>>> Since it's all unsigned arithmetic, it should be fine until 2106.
>>>>> However, we should get rid of get_seconds() long before then
>>>>> and use ktime_get_real_seconds() instead, as most other users
>>>>> of get_seconds() are (more) broken.
>>>>
>>>> Dtime on disk representation again breaks this for certain values in
>>>> 2038 even though everything is unsigned.
>>>>
>>>> I was just saying that whatever we do here depends on how dtime on
>>>> disk is interpreted.
>>>>
>>>> Agree that ktime_get_real_seconds() should be used here. But, the way
>>>> we handle new values would rely on this new interpretation of dtime.
>>>> Also, using time64_t variables on stack only matters after this. Once
>>>> the types are corrected, maybe the comparison expression need not
>>>> change at all (after new dtime interpretation is in place).
>>>
>>> There will not be a new dtime format on disk, but since the calculation
>>> here only depends on relative times (within a few minutes), then it would
>>> be fine to use only 32-bit timestamps, and truncate off the high bits
>>> from get_seconds()/ktime_get_real_seconds().
>>
>> Agreed.
>>
>> Are you planning to apply your fix for it then? I think your first
>> suggestion is all we need, aside from the three  minor comments
>> I had.
>
> Do you think it is worthwhile to introduce a "time_after32()" helper for this?
> I suspect that this will also be useful for other parts of the kernel that
> deal with relative 32-bit timestamps.

I can't think of any other one at the moment. The RTC code may need a
similar check somewhere but it's more likely that they want something
slightly different.

No objections to introducing a time_after32() from my side if only
for documentation purposes, but we probably won't use it elsewhere.

       Arnd