From: Hyunjung Ko atcrtc_alarm_isr() schedules atcrtc_dev->rtc_work (atcrtc_alarm_clear), whose handler obtains the atcrtc_dev via container_of() and dereferences rtc->rtc_dev (guard(rtc_ops)(rtc->rtc_dev)). atcrtc_probe() requested the alarm IRQ with devm_request_irq() *before* allocating rtc_dev with devm_rtc_allocate_device(), and INIT_WORK()'d rtc_work last with no teardown cancel. Because devres unwinds in reverse registration order, on unbind the RTC device is freed before the IRQ, and rtc_work is never cancelled at all: an alarm interrupt arriving around unbind can run atcrtc_alarm_clear() after rtc->rtc_dev (and the devm driver data) have been freed -> use-after-free. Allocate the RTC device first, then register rtc_work for device-managed cancellation with devm_work_autocancel(), and only then request the IRQ. devres now frees the IRQ first (so it can no longer schedule the work), then drains the work while rtc_dev is still valid, then frees rtc_dev and the driver data. Found by code review; not reproduced (requires ATCRTC100 hardware and an alarm interrupt racing device unbind). Fixes: 7adca706fe16 ("rtc: atcrtc100: Add ATCRTC100 RTC driver") Cc: stable@vger.kernel.org Signed-off-by: Hyunjung Ko --- drivers/rtc/rtc-atcrtc100.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/rtc/rtc-atcrtc100.c b/drivers/rtc/rtc-atcrtc100.c index 2433df3..51a38f0 100644 --- a/drivers/rtc/rtc-atcrtc100.c +++ b/drivers/rtc/rtc-atcrtc100.c @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -301,6 +302,26 @@ static int atcrtc_probe(struct platform_device *pdev) "Failed to get IRQ for alarm\n"); atcrtc_dev->alarm_irq = ret; + atcrtc_dev->rtc_dev = devm_rtc_allocate_device(&pdev->dev); + if (IS_ERR(atcrtc_dev->rtc_dev)) + return dev_err_probe(&pdev->dev, PTR_ERR(atcrtc_dev->rtc_dev), + "Failed to allocate RTC device\n"); + + set_bit(RTC_FEATURE_ALARM, atcrtc_dev->rtc_dev->features); + atcrtc_dev->rtc_dev->ops = &rtc_ops; + + /* + * Register rtc_work for device-managed cancellation after the RTC + * device it dereferences and before the IRQ that schedules it, so on + * unbind devres frees the IRQ, then drains the work, then frees the + * RTC device and driver data -- never the reverse (atcrtc_alarm_clear() + * touches rtc->rtc_dev). + */ + ret = devm_work_autocancel(&pdev->dev, &atcrtc_dev->rtc_work, + atcrtc_alarm_clear); + if (ret) + return ret; + ret = devm_request_irq(&pdev->dev, atcrtc_dev->alarm_irq, atcrtc_alarm_isr, @@ -312,12 +333,6 @@ static int atcrtc_probe(struct platform_device *pdev) "Failed to request IRQ %d for alarm\n", atcrtc_dev->alarm_irq); - atcrtc_dev->rtc_dev = devm_rtc_allocate_device(&pdev->dev); - if (IS_ERR(atcrtc_dev->rtc_dev)) - return dev_err_probe(&pdev->dev, PTR_ERR(atcrtc_dev->rtc_dev), - "Failed to allocate RTC device\n"); - - set_bit(RTC_FEATURE_ALARM, atcrtc_dev->rtc_dev->features); ret = device_init_wakeup(&pdev->dev, true); if (ret) return dev_err_probe(&pdev->dev, ret, @@ -330,9 +345,6 @@ static int atcrtc_probe(struct platform_device *pdev) "Failed to set wake IRQ\n"); } - atcrtc_dev->rtc_dev->ops = &rtc_ops; - - INIT_WORK(&atcrtc_dev->rtc_work, atcrtc_alarm_clear); return devm_rtc_register_device(atcrtc_dev->rtc_dev); } -- 2.43.0