Re: [PATCH 23/23] driver core: platform: count references to all kinds of firmware nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditionally putting `dev->fwnode` in `platform_device_release()` causes refcount underflows and use-after-free bugs for dynamically allocated fwnodes manually assigned by drivers.
- [Medium] The firmware node reference acquired during `platform_device_register_full()` is leaked on the error path because `ACPI_COMPANION_SET()` clears the `fwnode` pointer before it can be released.
--

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

driver core: platform: count references to all kinds of firmware nodes

This commit switches the platform device core to manage references for
all firmware nodes uniformly, rather than just OF nodes. It updates
platform_device_release() and assignment functions to use fwnode APIs.

> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -599,7 +599,7 @@ static void platform_device_release(struct device *dev)
>  	struct platform_object *pa = container_of(dev, struct platform_object,
>  						  pdev.dev);
>  
> -	of_node_put(pa->pdev.dev.of_node);
> +	fwnode_handle_put(pa->pdev.dev.fwnode);

[Severity: High]
If drivers manually assign their parent's fwnode during probe without
incrementing the reference count, will this regression cause a reference
count underflow?

For instance, some drivers assign the fwnode directly:

device_set_node(&pdev->dev, dev_fwnode(pdev->dev.parent));

Previously, of_node_put() was a safe no-op for non-OF nodes like software
nodes. Now, platform_device_release() unconditionally calls
fwnode_handle_put().

If the parent's fwnode is a dynamically allocated software node,
software_node_put() will be invoked without a corresponding
software_node_get().

Could this prematurely free the node and lead to a use-after-free for the
parent and other devices sharing it?

[ ... ]

> @@ -921,8 +918,8 @@ struct platform_device *platform_device_register_full(const struct platform_devi
>  		return ERR_PTR(-ENOMEM);
>  
>  	pdev->dev.parent = pdevinfo->parent;
> -	pdev->dev.fwnode = pdevinfo->fwnode;
> -	pdev->dev.of_node = of_node_get(to_of_node(pdev->dev.fwnode));
> +	pdev->dev.fwnode = fwnode_handle_get(pdevinfo->fwnode);
> +	pdev->dev.of_node = to_of_node(pdev->dev.fwnode);
>  	dev_assign_of_node_reused(&pdev->dev, pdevinfo->of_node_reused);
>  
>  	if (pdevinfo->dma_mask) {

[Severity: Medium]
Does this code leak the firmware node reference if an error occurs later in
this function?

If platform_device_add() fails, the error path executes:

err:
        ACPI_COMPANION_SET(&pdev->dev, NULL);
        platform_device_put(pdev);

The ACPI_COMPANION_SET() macro expands to set_primary_fwnode() and clears
pdev->dev.fwnode.

When platform_device_put() eventually calls platform_device_release(), the
fwnode pointer is already NULL.

Since fwnode_handle_put() acts as a no-op on a NULL pointer, will the
reference acquired here by fwnode_handle_get() be left unbalanced?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-pdev-fwnode-ref-v1-0-88c324a1b8d2@oss.qualcomm.com?part=23