Re: [PATCH v8 7/8] PCI: of: Set fwnode device of newly created PCI device nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dangling `fwnode->dev` pointer leads to Use-After-Free during PCI device or host bridge removal.
--

commit 282379de7d271ef07bfe62ea73e0c684092059cd
Author: Herve Codina <herve.codina@bootlin.com>

PCI: of: Set fwnode device of newly created PCI device nodes

This commit sets the fwnode device field for dynamically created PCI
device nodes. This ensures fw_devlink uses the correct PCI device as a
supplier rather than walking up to the PCI root bridge device.

> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index ee9eb384b377a..eda14cefca5e7 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -709,6 +709,13 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
>  	if (ret)
>  		goto out_free_node;
>  
> +	/*
> +	 * Set the fwnode device in order to have fw_devlink creating links
> +	 * pointing to this PCI device instead of walking up to the PCI host
> +	 * bridge.
> +	 */
> +	fw_devlink_set_device(&np->fwnode, &pdev->dev);

[Severity: High]
Could this leave a dangling pointer in np->fwnode.dev resulting in a 
use-after-free during device removal?

When of_pci_remove_node() is called during teardown, it executes 
device_remove_of_node(&pdev->dev) which clears pdev->dev.fwnode early.

When device_del() runs later, its standard cleanup logic relies on the
fwnode pointer still being intact on the device:

    if (dev->fwnode && dev->fwnode->dev == dev)
        fw_devlink_set_device(dev->fwnode, NULL);

Since pdev->dev.fwnode is already NULL from the earlier call, this cleanup 
is bypassed. If any reference to the OF node is held after the PCI device 
is freed, np->fwnode.dev will still point to the freed pdev->dev structure.

Does of_pci_remove_node() (and similarly of_pci_remove_host_bridge_node()) 
need to manually call fw_devlink_set_device(&np->fwnode, NULL) to clear 
this safely before the node is removed?

> +
>  	ret = of_changeset_apply(cset);
>  	if (ret)
>  		goto out_free_node;

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