linux-kernel - Re: [PATCH] rtc: adapt allowed RTC update error

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]

Date:   Thu, 3 Dec 2020 23:00:27 +0100
From:   Alexandre Belloni <alexandre.belloni@...tlin.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Jason Gunthorpe <jgg@...pe.ca>,
        Miroslav Lichvar <mlichvar@...hat.com>,
        linux-kernel@...r.kernel.org, John Stultz <john.stultz@...aro.org>,
        Prarit Bhargava <prarit@...hat.com>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        linux-rtc@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] rtc: adapt allowed RTC update error

On 03/12/2020 22:05:09+0100, Thomas Gleixner wrote:
> 2) I2C/SPI ...
> 
>    tsched t0                 t1                     t2
>           transfer(newsec)   RTC update (newsec)    RTC increments seconds
> 
>    Lets assume that ttransfer = t1 - t0 is known.

Note that ttransfer is one of the reason why setting set_offset_nsec
from the RTC driver is not a good idea. The same RTC may be on busses
with different rates and there is no way to know that. I think that was
one of my objections at the time.

ttransfer is not a function of the RTC model but rather of how it is
integrated in the system.

> 
>    tinc is the same as above = t2 - t1
> 
>    Again, lets assume that the event is accurate for now and ignore the fuzz
>    logic, i.e. tsched == t0
> 
>    So tsched has to be ttot = t2 - t0 _before_ wallclock reaches t2 and
>    increments seconds.
> 

I had a rough week and I'm probably not awake enough to follow
completely but is that thinking correct?

For the mc146818, you have t1 - t0 which is probably negligible and t2 -
T& == 500 ms

For most of the other RTCs, you have t1 - t0 is somewhat important,
probably around 100 to 150µs and t2 - t1 is 1s. I would think that what
is needed is tsched has to be t1-t0 before wallclock reaches t1. In that
case t2 doesn't matter, it will always be 1s after t1.

>    In this case newsec = t1.tv_sec = (t0 + ttransfer).tv_sec
> 
>    So now the fuzz logic for this is:
> 
>       newtime = t0 + ttransfer;
> 
>       if (newtime.tv_nsec < FUZZ)
>           newsec = newtime.tv_sec;
>       else if (newtime.tv_nsec > NSEC_PER_SEC - FUZZ)
>           newsec = newtime.tv_sec + 1;
>       else
>           goto fail;
> 
>    Again the first condition handles the case where t1 >= tsched and the
>    second one where t1 < tsched.
> 
> So now we have two options to fix this:
> 
>    1) Use a negative sync_offset for devices which need #1 above
>       (rtc_cmos & similar)
> 
>       That requires setting tsched to t2 - abs(sync_offset)
> 
>    2) Use always a positive sync_offset and a flag which tells
>       rtc_tv_nsec_ok() whether it needs to add or subtract.
> 
> #1 is good enough. All it takes is a comment at the timer start code why
> abs() is required.
> 
> Let me hack that up along with the hrtimer muck.
> 
> Thanks,
> 
>         tglx

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com