[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2d139619-455d-412f-d60b-e8d9259ed7e7@o2.pl>
Date: Thu, 31 Mar 2022 21:52:09 +0200
From: Mateusz Jończyk <mat.jonczyk@...pl>
To: Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc: linux-kernel@...r.kernel.org, linux-rtc@...r.kernel.org,
linux-kselftest@...r.kernel.org,
Alessandro Zummo <a.zummo@...ertech.it>,
Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH 1/2] [RFC] rtc: expose direct access to hardware alarm
time in debugfs
W dniu 31.03.2022 o 21:36, Alexandre Belloni pisze:
> Hello,
>
> On 31/03/2022 21:06:11+0200, Mateusz Jończyk wrote:
>> Before Linux 5.17, there was a problem with the CMOS RTC driver:
>> cmos_read_alarm() and cmos_set_alarm() did not check for the UIP (Update
>> in progress) bit, which could have caused it to sometimes fail silently
>> and read bogus values or do not set the alarm correctly.
>> Luckily, this issue was masked by cmos_read_time() invocations in core
>> RTC code - see https://marc.info/?l=linux-rtc&m=164858416511425&w=4
>>
>> To avoid such a problem in the future in some other driver, I wrote a
>> test unit that reads the alarm time many times in a row. As the alarm
>> time is usually read once and cached by the RTC core, this requires a
>> way for userspace to trigger direct alarm time read from hardware. I
>> think that debugfs is the natural choice for this.
>>
>> So, introduce /sys/kernel/debug/rtc/rtcX/wakealarm_raw. This interface
>> as implemented here does not seem to be that useful to userspace, so
>> there is little risk that it will become kernel ABI.
>>
>> Is this approach correct and worth it?
>>
> I'm not really in favor of adding another interface for very little
> gain, you want to use this interface to exercise the API in a way that
> will never happen in the real world, especially since __rtc_read_alarm
> is only called once, at registration time.
>
> I'm not sure the selftest is worth it then. You should better improve
> the existing unit tests by exercising the ioctls a bit more. syzbot did
> report interesting race conditions that were more severe.
OK, I did not know if other RTC drivers are likely to suffer from this kind of bugs.
I also thought that the bugs in cmos_read_alarm() / cmos_set_alarm() were more severe and
likely to affect existing users.
I had doubts if it's worth it, so I didn't finish the patches and sent it as RFC. It was a nice project, though.
Would you point to these race conditions reported by syzbot? I cannot find them.
Greetings,
Mateusz
Powered by blists - more mailing lists