On 12/11/24 18:50, Joe Hattori wrote:
Current code leaves the device's wakeup enabled in .remove() and in the
error path of .probe(), which results in a memory leak. Therefore, add
the device_init_wakeup(dev, false) call.
This bug was found by an experimental static analysis tool that I am
developing.
Fixes: 2cd98b22c144 ("rtc: brcmstb-waketimer: non-functional code changes")
Signed-off-by: Joe Hattori <joe@xxxxxxxxxxxxxxxxxxxxx>
Looks like you forgot to copy the maintainer (Doug Berger), added him.
--- drivers/rtc/rtc-brcmstb-waketimer.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c index fb47c32ab5ff..1bdfec94c693 100644 --- a/drivers/rtc/rtc-brcmstb-waketimer.c +++ b/drivers/rtc/rtc-brcmstb-waketimer.c @@ -298,15 +298,17 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev) device_init_wakeup(dev, true);ret = platform_get_irq(pdev, 0);- if (ret < 0) - return -ENODEV; + if (ret < 0) { + ret = -ENODEV; + goto err_disable_wakeup; + } timer->wake_irq = (unsigned int)ret;timer->clk = devm_clk_get(dev, NULL);if (!IS_ERR(timer->clk)) { ret = clk_prepare_enable(timer->clk); if (ret) - return ret; + goto err_disable_wakeup; timer->rate = clk_get_rate(timer->clk); if (!timer->rate) timer->rate = BRCMSTB_WKTMR_DEFAULT_FREQ; @@ -351,6 +353,8 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev) err_clk: clk_disable_unprepare(timer->clk);+err_disable_wakeup:+ device_init_wakeup(dev, false); return ret;
The error path change looks good to me,
}@@ -360,6 +364,7 @@ static void brcmstb_waketmr_remove(struct platform_device *pdev) unregister_reboot_notifier(&timer->reboot_notifier);clk_disable_unprepare(timer->clk); + device_init_wakeup(&pdev->dev, false);
That part, I don't think is necessary at all, since the interrupts handlers have been freed and disabled, they will not be causing any system wake-up, besides that, the device is completely gone from /sys.
-- Florian
