drivers/soc/microchip/mpfs-sys-controller.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
When platform_device_register() fails in mpfs_sys_controller_probe(),
the embedded struct device in subdevs[i] has already been initialized by
device_initialize(), but the failure path only reports the error and
does not drop the device reference for the current platform device:
mpfs_sys_controller_probe()
-> platform_device_register(&subdevs[i])
-> device_initialize(&subdevs[i].dev)
-> setup_pdev_dma_masks(&subdevs[i])
-> platform_device_add(&subdevs[i])
This leads to a reference leak when platform_device_register() fails.
Fix this by calling platform_device_put() after reporting the error.
The issue was identified by a static analysis tool I developed and
confirmed by manual review.
Fixes: d0054a470c339 ("soc: add microchip polarfire soc system controller")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/soc/microchip/mpfs-sys-controller.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/microchip/mpfs-sys-controller.c b/drivers/soc/microchip/mpfs-sys-controller.c
index 10b2fc39da66..404c31daf459 100644
--- a/drivers/soc/microchip/mpfs-sys-controller.c
+++ b/drivers/soc/microchip/mpfs-sys-controller.c
@@ -168,8 +168,10 @@ static int mpfs_sys_controller_probe(struct platform_device *pdev)
for (i = 0; i < ARRAY_SIZE(subdevs); i++) {
subdevs[i].dev.parent = dev;
- if (platform_device_register(&subdevs[i]))
+ if (platform_device_register(&subdevs[i])) {
dev_warn(dev, "Error registering sub device %s\n", subdevs[i].name);
+ platform_device_put(&subdevs[i]);
+ }
}
dev_info(&pdev->dev, "Registered MPFS system controller\n");
--
2.43.0
On Thu, Apr 16, 2026 at 02:16:35AM +0800, Guangshuo Li wrote:
> When platform_device_register() fails in mpfs_sys_controller_probe(),
> the embedded struct device in subdevs[i] has already been initialized by
> device_initialize(), but the failure path only reports the error and
> does not drop the device reference for the current platform device:
Patch looks reasonable, but not urgent so I will pick it up after -rc1.
Looking around it doesn't look like this will be a unique patch, so for
other I would suggest that...
> mpfs_sys_controller_probe()
> -> platform_device_register(&subdevs[i])
> -> device_initialize(&subdevs[i].dev)
> -> setup_pdev_dma_masks(&subdevs[i])
> -> platform_device_add(&subdevs[i])
...you redo this section, as it's not clear to me what this actually
is trying to communicate. AFAIU, what's wrong here is that
device_initialize() calls kobject_init(), which needs a kobject_put()
to clean up after it on failure. But this code snippet doesn't tell me
that, I had to go look for where the reference count was actually
incremented.
>
> This leads to a reference leak when platform_device_register() fails.
> Fix this by calling platform_device_put() after reporting the error.
>
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
>
> Fixes: d0054a470c339 ("soc: add microchip polarfire soc system controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/soc/microchip/mpfs-sys-controller.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/microchip/mpfs-sys-controller.c b/drivers/soc/microchip/mpfs-sys-controller.c
> index 10b2fc39da66..404c31daf459 100644
> --- a/drivers/soc/microchip/mpfs-sys-controller.c
> +++ b/drivers/soc/microchip/mpfs-sys-controller.c
> @@ -168,8 +168,10 @@ static int mpfs_sys_controller_probe(struct platform_device *pdev)
>
> for (i = 0; i < ARRAY_SIZE(subdevs); i++) {
> subdevs[i].dev.parent = dev;
> - if (platform_device_register(&subdevs[i]))
> + if (platform_device_register(&subdevs[i])) {
> dev_warn(dev, "Error registering sub device %s\n", subdevs[i].name);
> + platform_device_put(&subdevs[i]);
> + }
> }
>
> dev_info(&pdev->dev, "Registered MPFS system controller\n");
> --
> 2.43.0
>
Hi Conor, Thanks for the review. On Thu, 16 Apr 2026 at 21:32, Conor Dooley <conor@kernel.org> wrote: > > Patch looks reasonable, but not urgent so I will pick it up after -rc1. > Looking around it doesn't look like this will be a unique patch, so for > other I would suggest that... > > > mpfs_sys_controller_probe() > > -> platform_device_register(&subdevs[i]) > > -> device_initialize(&subdevs[i].dev) > > -> setup_pdev_dma_masks(&subdevs[i]) > > -> platform_device_add(&subdevs[i]) > > ...you redo this section, as it's not clear to me what this actually > is trying to communicate. AFAIU, what's wrong here is that > device_initialize() calls kobject_init(), which needs a kobject_put() > to clean up after it on failure. But this code snippet doesn't tell me > that, I had to go look for where the reference count was actually > incremented. > I also think your point makes sense. While this patch may look reasonable at the caller side, the correct fix may not be to handle individual callers one by one. It may be better to address this in platform_device_register() itself if the failure semantics there are the real issue. We are currently discussing that possibility in another patch of the same kind, to see whether fixing the API/core code would be a better approach than patching each caller separately: https://patchew.org/linux/20260415174159.3625777-1-lgs201920130244@gmail.com/ Thanks, Guangshuo
© 2016 - 2026 Red Hat, Inc.