Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

John Stultz
2016-06-21T17:13:59Z

On Tue, Jun 21, 2016 at 9:51 AM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Dienstag, 21. Juni 2016, 09:47:23 schrieb John Stultz:
>
> Hi John,
>
>> On Tue, Jun 21, 2016 at 9:34 AM, Stephan Mueller <smueller@chronox.de>
> wrote:
>> > Am Dienstag, 21. Juni 2016, 09:22:31 schrieb John Stultz:
>> >
>> > Hi John,
>> >
>> >> On Tue, Jun 21, 2016 at 1:32 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >> > On Tuesday, June 21, 2016 8:20:10 AM CEST Stephan Mueller wrote:
>> >> >> Am Freitag, 17. Juni 2016, 17:59:41 schrieb Arnd Bergmann:
>> >> > Compared to the previous __getnstimeofday(), the difference is
>> >> >
>> >> > - using "monotonic" timebase instead of "real", so the zero time
>> >> >
>> >> > is when the system booted rather than Jan 1 1970
>> >>
>> >> I haven't looked at the details of the calling code, but I'd worry for
>> >> crypto uses, especially if its being used for entropy collection,
>> >> using the monotonic clock instead of the realtime clock might be
>> >> problematic.
>> >
>> > Funnily it does not seem like that. All tests that I have conducted show
>> > that monotonic clocks behave equally as realtime clocks, because the
>> > uncertainty lies in the execution time of a set of instructions. All we
>> > need to do is to measure it with a timer that has a resolution that
>> > allows detecting these variations.
>>
>> Ok. If you're only using it for interval measurements, then either way
>> shouldn't matter. I just wanted to make sure the entropy wasn't coming
>> from the actual time.
>>
>> >> > - "raw" means we don't honor updates for the rate based on ntp,
>> >> >
>> >> > which is probably better as the ntp state might be observable
>> >> > over the net (it probably doesn't matter, but it can't hurt)
>> >>
>> >> So... this feels like a very vague explanation, and the lack of
>> >> frequency correction here probably need a really good comment. Keeping
>> >> multiple time domains is usually asking for trouble, but we added the
>> >> MONOTONIC_RAW clock to address a few cases where people really wanted
>> >> an abstract hardware counter, which was unaffected by frequency
>> >> corrections. I'd really make sure its clear why this is what you want
>> >> vs the standard system time domain so we don't run into problems
>> >> understanding it later.
>> >
>> > Perfect, that is what I would be interested in.
>>
>> But documenting *why* clearly is the thing I'd very strongly suggest.
>> If we need to make some slight semantic change for whatever reason, I
>> don't want folks worried "we can't do that because the crypto code is
>> using it for voodoo".
>
> I hope my explanation is sufficient to not count as voodoo: I only need an
> interval measurement capability which has a sufficient high resolution similar
> or better than RDTSC on x86.

So this is definitely more clear then what was described earlier, and
worries me because on many x86 machines (though fewer I guess these
days then in the past) the clocksource will often not be the TSC (and
have lower resolution).

So you might boot w/ "clocksource=acpi_pm" or "clocksource=hpet" to
test these other possible clocksources.

However, this was always the case, so if if __getnstimeofday64()
worked before (though, its not clear why you were using the __ version
prior, since that's supposed to be an internal function), it uses the
same clocksource, so I suspect there won't be any functional
difference the the pre-existing code.

But please just make sure the reason why you're using that specific
interface is clearly documented in the code so we don't have to later
reverse engineer the intent.

thanks
-john