drivers/clocksource/timer-ixp4xx.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
ixp4xx_timer_probe() directly returns the result of
platform_device_register(&ixp4xx_watchdog_device). When registration
fails, the embedded struct device in ixp4xx_watchdog_device has already
been initialized by device_initialize(), but the failure path does not
drop the device reference, leading to a reference leak.
The issue was identified by a static analysis tool I developed and
confirmed by manual review. Fix this by calling platform_device_put()
when platform_device_register() fails.
Fixes: 21a0a29d16c67 ("watchdog: ixp4xx: Rewrite driver to use core")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/clocksource/timer-ixp4xx.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-ixp4xx.c b/drivers/clocksource/timer-ixp4xx.c
index 720ed70a2964..924dbd58c4da 100644
--- a/drivers/clocksource/timer-ixp4xx.c
+++ b/drivers/clocksource/timer-ixp4xx.c
@@ -239,11 +239,16 @@ static struct platform_device ixp4xx_watchdog_device = {
static int ixp4xx_timer_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ int ret;
/* Pass the base address as platform data and nothing else */
ixp4xx_watchdog_device.dev.platform_data = local_ixp4xx_timer->base;
ixp4xx_watchdog_device.dev.parent = dev;
- return platform_device_register(&ixp4xx_watchdog_device);
+ ret = platform_device_register(&ixp4xx_watchdog_device);
+ if (ret)
+ platform_device_put(&ixp4xx_watchdog_device);
+
+ return ret;
}
static const struct of_device_id ixp4xx_timer_dt_id[] = {
--
2.43.0
Hi Guangshuo, thanks for your patch! On Mon, Apr 13, 2026 at 5:47 PM Guangshuo Li <lgs201920130244@gmail.com> wrote: > ixp4xx_timer_probe() directly returns the result of > platform_device_register(&ixp4xx_watchdog_device). When registration > fails, the embedded struct device in ixp4xx_watchdog_device has already > been initialized by device_initialize(), but the failure path does not > drop the device reference, leading to a reference leak. (...) > - return platform_device_register(&ixp4xx_watchdog_device); > + ret = platform_device_register(&ixp4xx_watchdog_device); > + if (ret) > + platform_device_put(&ixp4xx_watchdog_device); If the problem in the description is indeed there, it seems the bug is inside platform_device_register(), surely a function returning an error code is supposed to clean up any resources it takes before returning an error. It seems wrong to try to fix this in all the consumers. Yours, Linus Walleij
On 4/19/26 13:22, Linus Walleij wrote: > Hi Guangshuo, > > thanks for your patch! > > On Mon, Apr 13, 2026 at 5:47 PM Guangshuo Li <lgs201920130244@gmail.com> wrote: > >> ixp4xx_timer_probe() directly returns the result of >> platform_device_register(&ixp4xx_watchdog_device). When registration >> fails, the embedded struct device in ixp4xx_watchdog_device has already >> been initialized by device_initialize(), but the failure path does not >> drop the device reference, leading to a reference leak. > (...) > >> - return platform_device_register(&ixp4xx_watchdog_device); >> + ret = platform_device_register(&ixp4xx_watchdog_device); >> + if (ret) >> + platform_device_put(&ixp4xx_watchdog_device); > > If the problem in the description is indeed there, it seems the bug > is inside platform_device_register(), surely a function returning an > error code is supposed to clean up any resources it takes before > returning an error. It seems wrong to try to fix this in all the > consumers. > From platform_device_register(): /** * platform_device_register - add a platform-level device * @pdev: platform device we're adding * * NOTE: _Never_ directly free @pdev after calling this function, even if it * returned an error! Always use platform_device_put() to give up the * reference initialised in this function instead. */ Not that any code actually does that as far as I can see, but isn't the above doing exactly what the comment suggests ? Thanks, Guenter
On Sun, Apr 19, 2026 at 11:08 PM Guenter Roeck <linux@roeck-us.net> wrote: > On 4/19/26 13:22, Linus Walleij wrote: > > Hi Guangshuo, > > > > thanks for your patch! > > > > On Mon, Apr 13, 2026 at 5:47 PM Guangshuo Li <lgs201920130244@gmail.com> wrote: > > > >> ixp4xx_timer_probe() directly returns the result of > >> platform_device_register(&ixp4xx_watchdog_device). When registration > >> fails, the embedded struct device in ixp4xx_watchdog_device has already > >> been initialized by device_initialize(), but the failure path does not > >> drop the device reference, leading to a reference leak. > > (...) > > > >> - return platform_device_register(&ixp4xx_watchdog_device); > >> + ret = platform_device_register(&ixp4xx_watchdog_device); > >> + if (ret) > >> + platform_device_put(&ixp4xx_watchdog_device); > > > > If the problem in the description is indeed there, it seems the bug > > is inside platform_device_register(), surely a function returning an > > error code is supposed to clean up any resources it takes before > > returning an error. It seems wrong to try to fix this in all the > > consumers. > > > > From platform_device_register(): > > /** > * platform_device_register - add a platform-level device > * @pdev: platform device we're adding > * > * NOTE: _Never_ directly free @pdev after calling this function, even if it > * returned an error! Always use platform_device_put() to give up the > * reference initialised in this function instead. > */ > > Not that any code actually does that as far as I can see, but isn't > the above doing exactly what the comment suggests ? Yeah and Johan Hovold wrote that comment and he usually knows what he's doing so let's go with this then, I'm convinced! Reviewed-by: Linus Walleij <linusw@kernel.org> Yours, Linus Walleij
Hi Linus, Guenter, Thanks for reviewing and discussing this. On Mon, 20 Apr 2026 at 05:34, Linus Walleij <linusw@kernel.org> wrote: > > On Sun, Apr 19, 2026 at 11:08 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 4/19/26 13:22, Linus Walleij wrote: > > > > Hi Guangshuo, > > > > > > thanks for your patch! > > > > > > On Mon, Apr 13, 2026 at 5:47 PM Guangshuo Li <lgs201920130244@gmail.com> wrote: > > > > > >> ixp4xx_timer_probe() directly returns the result of > > >> platform_device_register(&ixp4xx_watchdog_device). When registration > > >> fails, the embedded struct device in ixp4xx_watchdog_device has already > > >> been initialized by device_initialize(), but the failure path does not > > >> drop the device reference, leading to a reference leak. > > > (...) > > > > > >> - return platform_device_register(&ixp4xx_watchdog_device); > > >> + ret = platform_device_register(&ixp4xx_watchdog_device); > > >> + if (ret) > > >> + platform_device_put(&ixp4xx_watchdog_device); > > > > > > If the problem in the description is indeed there, it seems the bug > > > is inside platform_device_register(), surely a function returning an > > > error code is supposed to clean up any resources it takes before > > > returning an error. It seems wrong to try to fix this in all the > > > consumers. > > > > > > > From platform_device_register(): > > > > /** > > * platform_device_register - add a platform-level device > > * @pdev: platform device we're adding > > * > > * NOTE: _Never_ directly free @pdev after calling this function, even if it > > * returned an error! Always use platform_device_put() to give up the > > * reference initialised in this function instead. > > */ > > > > Not that any code actually does that as far as I can see, but isn't > > the above doing exactly what the comment suggests ? > > Yeah and Johan Hovold wrote that comment and he usually knows > what he's doing so let's go with this then, I'm convinced! > > Reviewed-by: Linus Walleij <linusw@kernel.org> > > Yours, > Linus Walleij After further checking, this patch is not appropriate for this driver. ixp4xx_watchdog_device is a static platform_device, and it does not have a dev.release callback. Calling platform_device_put() on the platform_device_register() failure path can therefore trigger the missing release callback warning. So please disregard this patch. I will drop it and will also go back and check the other patches I sent for the same pattern, and send follow-ups where they should be ignored or reverted. Sorry for the confusion, and thanks again for the review. Best regards, Guangshuo Li
© 2016 - 2026 Red Hat, Inc.