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
