Re: [PATCH v2 10/19] pmdomain: imx: use platform_device_set_of_node()

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] OF node reference leak introduced by replacing direct assignment with platform_device_set_of_node().

Pre-existing issues:
- [High] Missing cleanup of dynamically allocated child platform devices causes Use-After-Free on probe error and remove paths.
--

commit f80cbf40e80bb2e67469b487f2abcc9652b28cb1
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

pmdomain: imx: use platform_device_set_of_node()

Ahead of reworking the reference counting logic for platform devices,
encapsulate the assignment of the OF node for dynamically allocated
platform devices with the provided helper.

> diff --git a/drivers/pmdomain/imx/gpc.c b/drivers/pmdomain/imx/gpc.c
> index 42e50c9b4fb9f..abca5f449a226 100644
> --- a/drivers/pmdomain/imx/gpc.c
> +++ b/drivers/pmdomain/imx/gpc.c
> @@ -487,8 +487,7 @@ static int imx_gpc_probe(struct platform_device *pdev)
>  			domain->ipg_rate_mhz = ipg_rate_mhz;
>  
>  			pd_pdev->dev.parent = &pdev->dev;
> -			pd_pdev->dev.of_node = of_node_get(np);
> -			pd_pdev->dev.fwnode = of_fwnode_handle(np);
> +			platform_device_set_of_node(pd_pdev, np);

[Severity: Medium]
Does replacing direct assignment with platform_device_set_of_node()
introduce an OF node reference leak?

In drivers/i2c/busses/i2c-pxa-pci.c:add_i2c_device(), the function acquires
an OF node reference via the for_each_child_of_node iterator and then calls
platform_device_set_of_node(pdev, child) on success.

Before this patch series, pdev->dev.of_node = child directly transferred
ownership of the reference to the platform device, which released it upon
destruction. Because the new helper platform_device_set_of_node()
explicitly increments the reference count internally, and there is no
of_node_put(child) added after the helper call, does this permanently leak
the original iterator reference on both success and error paths?

>  
>  			ret = platform_device_add(pd_pdev);
>  			if (ret) {

[Severity: High]
This is a pre-existing issue, but does the missing cleanup of dynamically
allocated child platform devices cause a use-after-free on probe error and
remove paths?

In imx_gpc_probe(), child devices (pd_pdev) are dynamically allocated and
registered in a loop. If a subsequent iteration fails and returns here,
the function returns immediately without unregistering the previously added
children.

Furthermore, in imx_gpc_remove(), when using the new device tree binding
(pgc_node is present), it exits without unregistering any child devices.

Since the child devices remain registered, their bound drivers (imx-pgc-pd)
stay active. When the parent devres cleanup frees the regmap, won't any
subsequent power state transition by the active child domains dereference
the freed domain->regmap, resulting in a use-after-free crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-pdev-fwnode-ref-v2-0-8abe2513f96e@oss.qualcomm.com?part=10