Re: [PATCH v9 6/9] PCI: of: Use fw_devlink_set_device()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Dangling pointer in `np->fwnode.dev` on error paths and during bridge removal, leading to a potential use-after-free.
--

--- Patch [6]: [PATCH v9 6/9] PCI: of: Use fw_devlink_set_device() ---
commit e05d94ee3b1f4fa9d5ff1b29db6b40e43eca7313
Author: Herve Codina <herve.codina@bootlin.com>

PCI: of: Use fw_devlink_set_device()

The code directly set the fwnode.dev field. This commit updates the code
to use the dedicated fw_devlink_set_device() helper to perform this
operation.

> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 8b18c4ba845cf..ee9eb384b377a 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -803,7 +803,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
>  	 * bus. Avoid any new device creation.
>  	 */
>  	of_node_set_flag(np, OF_POPULATED);
> -	np->fwnode.dev = &bridge->dev;
> +	fw_devlink_set_device(&np->fwnode, &bridge->dev);

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to a
use-after-free on error paths?

In of_pci_make_host_bridge_node(), if an error occurs after setting up the
fwnode (such as of_changeset_apply() or device_add_of_node() failing), the
error path removes the node from the tree and releases its reference, but
fails to clear the np->fwnode.dev pointer:

out_revert_cset:
	np->data = NULL;
	of_changeset_revert(cset);
out_free_node:
	of_node_put(np);

Because device_remove_of_node() only clears the forward pointer
(dev->fwnode) and not the backpointer, np->fwnode.dev remains dangling. If
another thread holds a reference to np, the node outlives the function.
When the bridge is later freed, any concurrent access via
get_dev_from_fwnode(&np->fwnode) could dereference freed memory.

The same dangling pointer issue appears to exist in
of_pci_remove_host_bridge_node() during teardown:

void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge)
{
...
	device_remove_of_node(&bridge->bus->dev);
	device_remove_of_node(&bridge->dev);
	of_changeset_revert(np->data);
	of_changeset_destroy(np->data);
	of_node_put(np);
}

>  	fwnode_dev_initialized(&np->fwnode, true);
>  
>  	ret = of_changeset_apply(cset);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708100302.517792-1-herve.codina@bootlin.com?part=6