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