Re: [PATCH v10 08/10] PCI: of: Set fwnode device of newly created PCI device nodes

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

Pre-existing issues:
- [High] In `of_pci_remove_node`, a NULL pointer dereference or memory corruption occurs if the PCI device's device tree node was provided by a device tree overlay rather than `of_pci_make_dev_node`.
--

--- Patch [8]: [PATCH v10 08/10] PCI: of: Set fwnode device of newly created PCI device nodes ---
commit f4dcbb5e22115fd2e6073441c7d4ac5e66dae5c4
Author: Herve Codina <[email protected]>

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

This commit sets the fwnode device for PCI device nodes dynamically created 
by the PCI core. This ensures that fw_devlink creates correct links pointing
to the PCI device rather than walking up to the PCI host bridge when overlays
are applied.

> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 6a533f9bbf3c5..a98f360c289ae 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -657,6 +657,7 @@ void of_pci_remove_node(struct pci_dev *pdev)
>  	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
>  		return;
>  
> +	fw_devlink_set_device(&np->fwnode, NULL);
>  	device_remove_of_node(&pdev->dev);

[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer 
dereference or memory corruption if the device tree node was provided by 
a standard device tree overlay?

If a user applies a standard device tree overlay describing a PCI device, 
the newly created device node will have the OF_DYNAMIC flag set. When the 
PCI device is later removed (e.g., via PCIe hotplug or device unbind), 
of_pci_remove_node() will see the OF_DYNAMIC flag and execute these lines. 

However, since the node was created by a standard overlay and not 
by of_pci_make_dev_node(), np->data may be NULL or contain unrelated 
data. Is it safe to unconditionally pass np->data to of_changeset_revert() 
and of_changeset_destroy() here?

>  	of_changeset_revert(np->data);
>  	of_changeset_destroy(np->data);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8