linux-kernel - Re: [PATCH] kdb: use __ktime_get_real_seconds instead of __current_kernel

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:   Wed, 6 Dec 2017 15:50:14 -0600
From:   Jason Wessel <jason.wessel@...driver.com>
To:     Arnd Bergmann <arnd@...db.de>, Thomas Gleixner <tglx@...utronix.de>
CC:     <y2038@...ts.linaro.org>, John Stultz <john.stultz@...aro.org>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Deepa Dinamani <deepa.kernel@...il.com>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        <linux-kernel@...r.kernel.org>,
        <kgdb-bugreport@...ts.sourceforge.net>
Subject: Re: [PATCH] kdb: use __ktime_get_real_seconds instead of
 __current_kernel_time

On 10/12/2017 09:06 AM, Arnd Bergmann wrote:
> kdb is the only user of the __current_kernel_time() interface, which is
> not y2038 safe and should be removed at some point.
> 
> The kdb code also goes to great lengths to print the time in a
> human-readable format from 'struct timespec', again using a non-y2038-safe
> re-implementation of the generic time_to_tm() code.
> 
> Using __current_kernel_time() here is necessary since the regular
> accessors that require a sequence lock might hang when called during the
> xtime update. However, this is safe in the particular case since kdb is
> only interested in the tv_sec field that is updated atomically.
> 
> In order to make this y2038-safe, I'm converting the code to the generic
> time64_to_tm helper, but that introduces the problem that we have no
> interface like __current_kernel_time() that provides a 64-bit timestamp
> in a lockless, safe and architecture-independent way. I have multiple
> ideas for how to solve that:
> 
> - __ktime_get_real_seconds() is lockless, but can return
>    incorrect results on 32-bit architectures in the special case that
>    we are in the process of changing the time across the epoch, either
>    during the timer tick that overflows the seconds in 2038, or while
>    calling settimeofday.
> 
> - ktime_get_real_fast_ns() would work in this context, but does
>    require a call into the clocksource driver to return a high-resolution
>    timestamp. This may have undesired side-effects in the debugger,
>    since we want to limit the interactions with the rest of the kernel.
> 
> - Adding a ktime_get_real_fast_seconds() based on tk_fast_mono
>    plus tkr->base_real without the tk_clock_read() delta. Not sure about
>    the value of adding yet another interface here.
> 
> - Changing the existing ktime_get_real_seconds() to use
>    tk_fast_mono on 32-bit architectures rather than xtime_sec.  I think
>    this could work, but am not entirely sure if this is an improvement.
> 
> I picked the first of those for simplicity here. It's technically
> not correct but probably good enough as the time is only used for the
> debugging output and the race will likely never be hit in practice.
> Another downside is having to move the declaration into a public header
> file.
> 
> Let me know if anyone has a different preference.


It all seems reasonable to me.  Separately I created the same patch because I didn't see this mail first.  The only difference was that I added a comment about the __ktime_get_real_seconds() not taking the lock because it was done that way in other places in the header file.

===
  extern time64_t ktime_get_real_seconds(void);
+/* does not take xtime_lock */
+extern time64_t __ktime_get_real_seconds(void);
===

Acked-by: Jason Wessel <jason.wessel@...driver.com>

Thanks for your work on the 2038 problems.  :-)

Cheers,
Jason.