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

[Herve Codina <herve.codina@bootlin.com>]

Hi,

On Wed, 08 Jul 2026 10:29:21 +0000
sashiko-bot@kernel.org wrote:

> 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);
> }

This issue is relevant and clearing np->fwnode.dev in the error path will
be take into account in the next iteration.

Hervé