[PATCH] rtc: atcrtc100: cancel alarm work on remove

[PATCH] rtc: atcrtc100: cancel alarm work on remove

[Fan Wu]

The alarm interrupt handler atcrtc_alarm_isr() queues rtc_work on the
system workqueue to clear the alarm. The handler atcrtc_alarm_clear()
dereferences the devm-managed atcrtc_dev through container_of(), takes
rtc_lock() on the RTC device and writes the regmap.

The driver has no remove callback, and the devres cleanup only frees
the interrupt before the remaining resources. free_irq() waits for the
interrupt handler, but it does not cancel work the handler already
queued. A pending atcrtc_alarm_clear() can therefore run after the
regmap, the I/O mapping and finally the device structure have been
released, and dereference freed memory.

Add a remove callback that frees the interrupt first, so no new work
can be queued, and then cancels the alarm work, following the same
pattern as rtc-ds1374 and rtc-ds1305.

It also clears the wake IRQ and disables the wakeup source configured
in probe. These are not devres-managed, and the driver core only
releases them when the device itself is removed, not on unbind.
Without this cleanup, a later bind would fail probe with -EEXIST:
device_wakeup_attach() rejects a second wakeup source for the same
device, and the leftover wake IRQ would make dev_pm_set_wake_irq()
fail with -EEXIST and a WARN as well.

This issue was found by an in-house static analysis tool.

Fixes: 7adca706fe16 ("rtc: atcrtc100: Add ATCRTC100 RTC driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/rtc/rtc-atcrtc100.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
diff --git a/drivers/rtc/rtc-atcrtc100.c b/drivers/rtc/rtc-atcrtc100.c
index 9808fc2c5a49..3c7313218da5 100644
--- a/drivers/rtc/rtc-atcrtc100.c
+++ b/drivers/rtc/rtc-atcrtc100.c
@@ -337,6 +337,16 @@ static int atcrtc_probe(struct platform_device *pdev)
 	return devm_rtc_register_device(atcrtc_dev->rtc_dev);
 }
 
+static void atcrtc_remove(struct platform_device *pdev)
+{
+	struct atcrtc_dev *atcrtc_dev = platform_get_drvdata(pdev);
+
+	dev_pm_clear_wake_irq(&pdev->dev);
+	device_init_wakeup(&pdev->dev, false);
+	devm_free_irq(&pdev->dev, atcrtc_dev->alarm_irq, atcrtc_dev);
+	cancel_work_sync(&atcrtc_dev->rtc_work);
+}
+
 static int atcrtc_resume(struct device *dev)
 {
 	struct atcrtc_dev *rtc = dev_get_drvdata(dev);
@@ -372,6 +382,7 @@ static struct platform_driver atcrtc_platform_driver = {
 		.pm = pm_sleep_ptr(&atcrtc_pm_ops),
 	},
 	.probe = atcrtc_probe,
+	.remove = atcrtc_remove,
 };
 
 module_platform_driver(atcrtc_platform_driver);

Re: [PATCH] rtc: atcrtc100: cancel alarm work on remove

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] CRASH: NULL pointer dereference and uninitialized memory execution. The alarm IRQ is requested prematurely in probe before the required structures are initialized.
- [High] LEAK AND UAF: The probe function's error path for devm_rtc_register_device() fails to clean up newly configured wake capabilities and the workqueue.
--

commit 913e1e349945d1e573a4b95047a974a86f29a566
Author: Fan Wu <fanwu01@zju.edu.cn>

rtc: atcrtc100: cancel alarm work on remove

This commit adds a remove callback to the atcrtc100 RTC driver to properly
clean up resources. It clears the wake IRQ, disables wakeup, frees the
interrupt, and cancels the alarm work to prevent use-after-free and -EEXIST
issues on rebinding.

> diff --git a/drivers/rtc/rtc-atcrtc100.c b/drivers/rtc/rtc-atcrtc100.c
> index 9808fc2c5a492..3c7313218da51 100644
> --- a/drivers/rtc/rtc-atcrtc100.c
> +++ b/drivers/rtc/rtc-atcrtc100.c
> @@ -337,6 +337,16 @@ static int atcrtc_probe(struct platform_device *pdev)
>  	return devm_rtc_register_device(atcrtc_dev->rtc_dev);

[Severity: High]
This isn't a bug introduced by this patch, but does this error path leak
resources or risk a use-after-free?

If devm_rtc_register_device() fails in atcrtc_probe(), it returns the error
immediately:

atcrtc_probe() {
    ...
    return devm_rtc_register_device(atcrtc_dev->rtc_dev);
}

However, earlier in atcrtc_probe(), device_init_wakeup() and
dev_pm_set_wake_irq() are called, which are not devres-managed. Do these wake
configurations leak on this error path?

Also, if the alarm IRQ fired and scheduled rtc_work, wouldn't returning without
cancel_work_sync() leave the work pending? Once devres frees atcrtc_dev,
couldn't this lead to a use-after-free when the system workqueue eventually
executes the uncancelled work?

[Severity: High]
This is also a pre-existing issue, but is there a race condition when the alarm
IRQ is requested earlier in atcrtc_probe()?

The IRQ is requested via devm_request_irq() before atcrtc_dev->rtc_dev is
allocated and before INIT_WORK() initializes rtc_work:

atcrtc_probe() {
    ...
    ret = devm_request_irq(&pdev->dev, atcrtc_dev->alarm_irq, ...);
    ...
    atcrtc_dev->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
    ...
    INIT_WORK(&atcrtc_dev->rtc_work, atcrtc_alarm_clear);
}

If the hardware interrupt fires immediately after devm_request_irq(), the
handler atcrtc_alarm_isr() will execute:

atcrtc_alarm_isr() {
    ...
    schedule_work(&rtc->rtc_work);
    rtc_update_irq(rtc->rtc_dev, 1, RTC_AF | RTC_IRQF);
    ...
}

Will this cause schedule_work() to operate on uninitialized memory and
rtc_update_irq() to dereference a NULL pointer? Should the IRQ request be moved
until after these structures are fully initialized?

>  }
>  
> +static void atcrtc_remove(struct platform_device *pdev)
> +{
> +	struct atcrtc_dev *atcrtc_dev = platform_get_drvdata(pdev);
> +
> +	dev_pm_clear_wake_irq(&pdev->dev);
> +	device_init_wakeup(&pdev->dev, false);
> +	devm_free_irq(&pdev->dev, atcrtc_dev->alarm_irq, atcrtc_dev);
> +	cancel_work_sync(&atcrtc_dev->rtc_work);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909152651.712614-1-fanwu01@zju.edu.cn?part=1