linux-kernel - Re: [PATCH] rtc: ds1307: set uie_unsupported if no interrupt is available

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:   Tue, 30 Mar 2021 02:02:05 +0200
From:   Alexandre Belloni <alexandre.belloni@...tlin.com>
To:     Lukasz Stelmach <l.stelmach@...sung.com>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>, linux-rtc@...r.kernel.org,
        Bartłomiej Żolnierkiewicz 
        <b.zolnierkie@...sung.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rtc: ds1307: set uie_unsupported if no interrupt is
 available

On 16/03/2021 19:04:14+0100, Lukasz Stelmach wrote:
> OK, you are right. The problem seems to be elsewhere.
> 
> How about this scnario? We call rtc_update_irq_enable(). We read rtc
> with __rtc_read_time() and calculate the alarm time. We get through
> rtc_timer_enqueue() and down to __rtc_set_alarm(). We loose the race
> condition (I can do it, I've got really slow connection to DS3231) and
> we return -ETIME from __rtc_set_alarm()
> 
>     if (scheduled <= now)
>         return -ETIME;
> 
> and 0 from rtc_timer_enqueue() and the very same zero from
> rtc_update_irq_enable(). The caller of ioctl() thinks they can expect
> interrupts when, in fact, they won't receive any.
> 
> The really weird stuff happens in rtc_timer_do_work(). For the timer to
> be dequeued __rtc_set_alarm() needs to return EINVAL three times in a
> row. In my setup this doesn't happen and the code keeps running loops
> around "reporogram" and "again" labels.
> 
> With my patch we never risk the above race condition between
> __rtc_read_time() in rtc_update_irq_enable() and the one in
> __rtc_set_alarm(), because we know rtc doesn't support alarms before we
> start the race. In fact there is another race between __rtc_read_time()
> and actually setting the alarm in the chip.
> 
> IMHO the solution is to introduce RTC_HAS_ALARM flag for struct
> rtc_device and check it at the very beginning of __rtc_set_alarm() the
> same way it is being done in ds1337_set_alarm(). What are your thoughts?
> 

I did introduce RTC_FEATURE_ALARM for that in v5.12. I'm sending patches
that are not well tested but should solve your issue.


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