Re: [PATCH v13 08/10] 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] base: TOCTOU data race and UAF on fwnode->dev access
--

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

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

Device-tree node can be created when CONFIG_PCI_DYNAMIC_OF_NODES. Those
nodes are created and filled based on PCI core information but the
fwnode device field is not set.

> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 29b63a9090ae3..8914bfa25dc11 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -736,6 +736,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);

[Severity: High]
Does this locklessly clear the fwnode device while another traversal might be
reading it?

If a PCI device is concurrently removed while a driver is being bound to
another device on the same system, of_pci_remove_node() clears fwnode->dev
locklessly here.

Concurrently, __fw_devlink_pickup_dangling_consumers() in drivers/base/core.c
can traverse the fwnode tree and read it locklessly:

	if (fwnode->dev && fwnode->dev->bus)
		return;

Can this lead to a NULL pointer dereference if the compiler emits two loads
and the pointer is cleared in between?

Can this also cause a use-after-free if the pointer is read, then the device
is concurrently unregistered and freed before the bus field is accessed, as
no reference count is held during the lockless read?

>  	device_remove_of_node(&pdev->dev);
>  	of_changeset_revert(np->data);
>  	of_changeset_destroy(np->data);

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