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é