Skip to content

Commit 00a39d8

Browse files
matjonalexandrebelloni
authored andcommitted
rtc: cmos: use spin_lock_irqsave in cmos_interrupt
cmos_interrupt() can be called in a non-interrupt context, such as in an ACPI event handler (which runs in an interrupt thread). Therefore, usage of spin_lock(&rtc_lock) is insecure. Use spin_lock_irqsave() / spin_unlock_irqrestore() instead. Before a misguided commit 6950d04 ("rtc: cmos: Replace spin_lock_irqsave with spin_lock in hard IRQ") the cmos_interrupt() function used spin_lock_irqsave(). That commit changed it to spin_lock() and broke locking, which was partially fixed in commit 13be2ef ("rtc: cmos: Disable irq around direct invocation of cmos_interrupt()") That second commit did not take account of the ACPI fixed event handler pathway, however. It introduced local_irq_disable() workarounds in cmos_check_wkalrm(), which can cause problems on PREEMPT_RT kernels and are now unnecessary. Add an explicit comment so that this change will not be reverted by mistake. Cc: [email protected] Fixes: 6950d04 ("rtc: cmos: Replace spin_lock_irqsave with spin_lock in hard IRQ") Signed-off-by: Mateusz Jończyk <[email protected]> Reviewed-by: Sebastian Andrzej Siewior <[email protected]> Tested-by: Chris Bainbridge <[email protected]> Reported-by: Chris Bainbridge <[email protected]> Closes: https://lore.kernel.org/all/[email protected]/ Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexandre Belloni <[email protected]>
1 parent b1248da commit 00a39d8

File tree

1 file changed

+6
-4
lines changed

1 file changed

+6
-4
lines changed

drivers/rtc/rtc-cmos.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -692,8 +692,12 @@ static irqreturn_t cmos_interrupt(int irq, void *p)
692692
{
693693
u8 irqstat;
694694
u8 rtc_control;
695+
unsigned long flags;
695696

696-
spin_lock(&rtc_lock);
697+
/* We cannot use spin_lock() here, as cmos_interrupt() is also called
698+
* in a non-irq context.
699+
*/
700+
spin_lock_irqsave(&rtc_lock, flags);
697701

698702
/* When the HPET interrupt handler calls us, the interrupt
699703
* status is passed as arg1 instead of the irq number. But
@@ -727,7 +731,7 @@ static irqreturn_t cmos_interrupt(int irq, void *p)
727731
hpet_mask_rtc_irq_bit(RTC_AIE);
728732
CMOS_READ(RTC_INTR_FLAGS);
729733
}
730-
spin_unlock(&rtc_lock);
734+
spin_unlock_irqrestore(&rtc_lock, flags);
731735

732736
if (is_intr(irqstat)) {
733737
rtc_update_irq(p, 1, irqstat);
@@ -1295,9 +1299,7 @@ static void cmos_check_wkalrm(struct device *dev)
12951299
* ACK the rtc irq here
12961300
*/
12971301
if (t_now >= cmos->alarm_expires && cmos_use_acpi_alarm()) {
1298-
local_irq_disable();
12991302
cmos_interrupt(0, (void *)cmos->rtc);
1300-
local_irq_enable();
13011303
return;
13021304
}
13031305

0 commit comments

Comments
 (0)