Re: [RESEND PATCH 1/2] rtc: cmos: Disable ACPI RTC event before handling it

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rtc: cmos: TOCTOU race in ACPI RTC event toggle leads to lost alarms
--

commit 73bcc5b0dfcf900941896af4381bb8051c9589e0
Author: Gregory Price <gourry@gourry.net>

rtc: cmos: Disable ACPI RTC event before handling it

Users of RTC_UIE_ON can stop receiving update interrupts when rtc-cmos
handles alarms through ACPI. Clear and disable the current ACPI RTC event
before cmos_interrupt() schedules the work to avoid a race where the handler
undoes the worker's event enablement.

> diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> index fa04ece151b8b..fe514b1aa3433 100644
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -758,7 +758,9 @@ static u32 rtc_handler(void *context)
>  	unsigned char rtc_intr;
>  	unsigned long flags;
>  
> -
> +	/* Quiesce this event before rtc->irqwork can arm the next one. */
> +	acpi_clear_event(ACPI_EVENT_RTC);
> +	acpi_disable_event(ACPI_EVENT_RTC, 0);

[Severity: Medium]
Could this unconditional disablement create a race with concurrent userspace
threads setting a new alarm?

If a concurrent userspace thread calls ioctl(RTC_ALM_SET), it acquires the
necessary locks and enables the ACPI event for the newly configured alarm:

drivers/rtc/rtc-cmos.c:rtc_wake_on() {
        acpi_clear_event(ACPI_EVENT_RTC);
        acpi_enable_event(ACPI_EVENT_RTC, 0);
}

While the patch successfully resolves a similar race with the worker thread
by moving the disablement before cmos_interrupt(), it seems to leave the
asynchronous userspace race unhandled.

If the userspace thread enables the event after the SCI thread has been
triggered, but before the SCI thread executes acpi_disable_event() here in
rtc_handler(), will the SCI thread mistakenly disable the newly set alarm?

This could cause the newly configured alarm to be dropped, as the disablement
happens without holding locks such as rtc_lock or rtc->ops_lock.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015173029.3091872-1-gourry@gourry.net?part=1