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

Herve Codina <[email protected]>
Newsgroups org.kernel.vger.linux-cxl,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-pci
Organization Bootlin
Message-ID <[email protected]>
Hi,

On Wed, 08 Jul 2026 10:29:21 +0000
[email protected] 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 <[email protected]>
> 
> 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é
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.