[PATCH v10 00/10] lan966x pci device: Add support for SFPs, PCI part

[PATCH v10 00/10] lan966x pci device: Add support for SFPs, PCI part

[Herve Codina]

Hi,

Previously, I sent a big picture series adding support for SFP ports
available on the LAN966x PCI device [0].

In this series patches touch several parts and sub-system in the kernel.
Reviews have be done and it makes sense to split the series and send
parts separately.

This current series is the extraction of patches related to issues in
the PCI subsystem. It has to be seen as a continuation of the big
picture series but related to this specific core part.

Patches 1 to 6 introduce and use fw_devlink_set_device() in already
existing code. No functional change but the introduction of the
fw_devlink_set_device() wrapper.

Patch 7 avoid a dangling pointer.

Patches 8 and 9 fix fw_devlink issues specific to PCI and the
device-tree nodes created during enumeration.

Patch 10 adds a guards to prevent incorrect node removal.

[0] https://lore.kernel.org/all/20260325143555.451852-1-herve.codina@bootlin.com/

Best regards,
Hervé

Changes:

v9 -> v10
  v9: https://lore.kernel.org/all/20260708100302.517792-1-herve.codina@bootlin.com/

  - Patches 1 to 6
    No changes

  - Patch 7
    Clear fwnode->dev also on error path.

  - Patch 8
    Clear fwnode->dev also on error path.

  - Patch 9
    No changes

  - Patch 10 (New patch in v10)
    Guard against node removal with incorrect np->data.

v8 -> v9
  v8: https://lore.kernel.org/all/20260630102804.413563-1-herve.codina@bootlin.com/

  - Patch 7 (new patch in v9)
    Clear fwnode->dev during root bridge node removal (avoid a dangling
    pointer).

  - Patch 8 (patch 7 in v8)
    Clear fwnode->dev during PCI device node removal.

v7 -> v8
  v7: https://lore.kernel.org/all/20260511155930.34604-1-herve.codina@bootlin.com/

  Rebase on top of v7.2-rc1

  - patches 4 and 5
    Add 'Reviewed-by: Andy Shevchenko'

  - Other Patches
    No other changes

v6 -> v7

  Rebase on top of v7.1-rc1

  - Patch 1
     Add Cc stable. Needed for patches 11 and 12

  - Patches 2 to 5
    No changes

  - Patch 6
     Add Cc stable. Needed for patch 13
     Add 'Acked-by: Bjorn Helgaas'

  - Patch 7
     Fix a typo in commit log
     Add Fixes tag + Cc stable
     Add 'Acked-by: Bjorn Helgaas'

  - Patch 8
     Add Fixes tag + Cc stable
     Add 'Acked-by: Bjorn Helgaas'
Older iterations:
  Patches 6 to 13 in the big picture series
  https://lore.kernel.org/all/20260325143555.451852-1-herve.codina@bootlin.com/

Herve Codina (10):
  driver core: fw_devlink: Introduce fw_devlink_set_device()
  drivers: core: Use fw_devlink_set_device()
  pinctrl: cs42l43: Use fw_devlink_set_device()
  cxl/test: Use device_set_node()
  cxl/test: Use fw_devlink_set_device()
  PCI: of: Use fw_devlink_set_device()
  PCI: of: Clear fwnode->dev during root bridge node removal and error
    path
  PCI: of: Set fwnode device of newly created PCI device nodes
  PCI: of: Remove fwnode_dev_initialized() call for a PCI root bridge
    node
  PCI: of: Guard against node removal with incorrect np->data

 drivers/base/core.c                      |  4 ++--
 drivers/pci/of.c                         | 24 ++++++++++++++++++------
 drivers/pinctrl/cirrus/pinctrl-cs42l43.c |  2 +-
 include/linux/fwnode.h                   |  6 ++++++
 tools/testing/cxl/test/cxl.c             |  4 ++--
 5 files changed, 29 insertions(+), 11 deletions(-)

-- 
2.55.0

[PATCH v10 01/10] driver core: fw_devlink: Introduce fw_devlink_set_device()

[Herve Codina]

Setting fwnode->dev is specific to fw_devlink.

In order to avoid having a direct 'fwnode->dev = dev;' in several
place in the kernel, introduce fw_devlink_set_device() helper to perform
this operation.

Having this helper allows to hide the fwnode devlink related stuff
behind the helper.

Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
Cc stable because used by other patches with Fixes + Cc stable
---
 include/linux/fwnode.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 4e86e6990d28..31538f54b1ca 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -256,4 +256,10 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
 void fw_devlink_refresh_fwnode(struct fwnode_handle *fwnode);
 bool fw_devlink_is_strict(void);
 
+static inline void fw_devlink_set_device(struct fwnode_handle *fwnode,
+					 struct device *dev)
+{
+	fwnode->dev = dev;
+}
+
 #endif
-- 
2.55.0

Re: [PATCH v10 01/10] driver core: fw_devlink: Introduce fw_devlink_set_device()

[sashiko-bot]

> Setting fwnode->dev is specific to fw_devlink.
> 
> In order to avoid having a direct 'fwnode->dev = dev;' in several
> place in the kernel, introduce fw_devlink_set_device() helper to perform
> this operation.
> 
> Having this helper allows to hide the fwnode devlink related stuff
> behind the helper.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCH v10 02/10] drivers: core: Use fw_devlink_set_device()

[Herve Codina]

The code set directly fwnode->dev field.

Use the dedicated fw_devlink_set_device() helper to perform this
operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 4d026682944f..c205125344cf 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3750,7 +3750,7 @@ int device_add(struct device *dev)
 	 * device and the driver sync_state callback is called for this device.
 	 */
 	if (dev->fwnode && !dev->fwnode->dev) {
-		dev->fwnode->dev = dev;
+		fw_devlink_set_device(dev->fwnode, dev);
 		fw_devlink_link_device(dev);
 	}
 
@@ -3925,7 +3925,7 @@ void device_del(struct device *dev)
 	device_unlock(dev);
 
 	if (dev->fwnode && dev->fwnode->dev == dev)
-		dev->fwnode->dev = NULL;
+		fw_devlink_set_device(dev->fwnode, NULL);
 
 	/* Notify clients of device removal.  This call must come
 	 * before dpm_sysfs_remove().
-- 
2.55.0

Re: [PATCH v10 02/10] drivers: core: Use fw_devlink_set_device()

[sashiko-bot]

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

Pre-existing issues:
- [High] Lockless read of `fwnode->dev` can lead to Use-After-Free and NULL pointer dereferences.
--

--- Patch [2]: [PATCH v10 02/10] drivers: core: Use fw_devlink_set_device() ---
commit 80a2fb2740f34981de6ee4d63b13800473b38e85
Author: Herve Codina <herve.codina@bootlin.com>

drivers: core: Use fw_devlink_set_device()

This patch updates the driver core to use the dedicated
fw_devlink_set_device() helper instead of assigning the fwnode->dev field
directly.

> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 4d026682944f2..c205125344cf3 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
[ ... ]
> @@ -3925,7 +3925,7 @@ void device_del(struct device *dev)
>  	device_unlock(dev);
>  
>  	if (dev->fwnode && dev->fwnode->dev == dev)
> -		dev->fwnode->dev = NULL;
> +		fw_devlink_set_device(dev->fwnode, NULL);
>  
>  	/* Notify clients of device removal.  This call must come
>  	 * before dpm_sysfs_remove().

[Severity: High]
This is a pre-existing issue, but does this lockless assignment expose a race
condition with concurrent readers?

In device_del(), fwnode->dev is cleared locklessly via
fw_devlink_set_device(). Concurrently, other operations holding
fwnode_link_lock (which doesn't serialize with device_del()) read fwnode->dev.

For example, if get_dev_from_fwnode() is called concurrently:

drivers/base/core.c:get_dev_from_fwnode() {
    ...
    return get_device((fwnode)->dev);
}

If the thread is preempted between reading fwnode->dev and dereferencing it
in get_device(), and device_del() finishes allowing the device to be freed,
could this result in a use-after-free?

Similarly, in __fw_devlink_pickup_dangling_consumers():

drivers/base/core.c:__fw_devlink_pickup_dangling_consumers() {
    ...
    if (fwnode->dev && fwnode->dev->bus)
    ...
}

Could the compiler reload fwnode->dev between the NULL check and the
dereference, leading to a NULL pointer dereference?

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

[PATCH v10 03/10] pinctrl: cs42l43: Use fw_devlink_set_device()

[Herve Codina]

The code set directly fwnode->dev field.

Use the dedicated fw_devlink_set_device() helper to perform this
operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Acked-by: Linus Walleij <linusw@kernel.org>
---
 drivers/pinctrl/cirrus/pinctrl-cs42l43.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/cirrus/pinctrl-cs42l43.c b/drivers/pinctrl/cirrus/pinctrl-cs42l43.c
index 8990fab0446c..563070aba3d8 100644
--- a/drivers/pinctrl/cirrus/pinctrl-cs42l43.c
+++ b/drivers/pinctrl/cirrus/pinctrl-cs42l43.c
@@ -576,7 +576,7 @@ static int cs42l43_pin_probe(struct platform_device *pdev)
 				return ret;
 
 			if (!child->dev)
-				child->dev = priv->dev;
+				fw_devlink_set_device(child, priv->dev);
 			fwnode = child;
 		}
 	}
-- 
2.55.0

Re: [PATCH v10 03/10] pinctrl: cs42l43: Use fw_devlink_set_device()

[sashiko-bot]

> The code set directly fwnode->dev field.
> 
> Use the dedicated fw_devlink_set_device() helper to perform this
> operation.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCH v10 04/10] cxl/test: Use device_set_node()

[Herve Codina]

The code set directly dev->fwnode.

Use the dedicated helper to perform this operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 tools/testing/cxl/test/cxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index ef92dd35e030..69da0727362b 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1528,7 +1528,7 @@ static void mock_companion(struct acpi_device *adev, struct device *dev)
 {
 	device_initialize(&adev->dev);
 	fwnode_init(&adev->fwnode, NULL);
-	dev->fwnode = &adev->fwnode;
+	device_set_node(dev, &adev->fwnode);
 	adev->fwnode.dev = dev;
 }
 
-- 
2.55.0

Re: [PATCH v10 04/10] cxl/test: Use device_set_node()

[sashiko-bot]

> The code set directly dev->fwnode.
> 
> Use the dedicated helper to perform this operation.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCH v10 05/10] cxl/test: Use fw_devlink_set_device()

[Herve Codina]

The code set directly fwnode.dev field.

Use the dedicated fw_devlink_set_device() helper to perform this
operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 tools/testing/cxl/test/cxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 69da0727362b..95e39dfc6ec9 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1529,7 +1529,7 @@ static void mock_companion(struct acpi_device *adev, struct device *dev)
 	device_initialize(&adev->dev);
 	fwnode_init(&adev->fwnode, NULL);
 	device_set_node(dev, &adev->fwnode);
-	adev->fwnode.dev = dev;
+	fw_devlink_set_device(&adev->fwnode, dev);
 }
 
 #ifndef SZ_64G
-- 
2.55.0

Re: [PATCH v10 05/10] cxl/test: Use fw_devlink_set_device()

[sashiko-bot]

> The code set directly fwnode.dev field.
> 
> Use the dedicated fw_devlink_set_device() helper to perform this
> operation.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCH v10 06/10] PCI: of: Use fw_devlink_set_device()

[Herve Codina]

The code set directly fwnode.dev field.

Use the dedicated fw_devlink_set_device() helper to perform this
operation.

Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
Cc stable because used by other patches with Fixes + Cc stable
---
 drivers/pci/of.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 8b18c4ba845c..ee9eb384b377 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);
 	fwnode_dev_initialized(&np->fwnode, true);
 
 	ret = of_changeset_apply(cset);
-- 
2.55.0

Re: [PATCH v10 06/10] PCI: of: Use fw_devlink_set_device()

[sashiko-bot]

> The code set directly fwnode.dev field.
> 
> Use the dedicated fw_devlink_set_device() helper to perform this
> operation.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCH v10 07/10] PCI: of: Clear fwnode->dev during root bridge node removal and error path

[Herve Codina]

During the of_pci_make_host_bridge_node() call, an OF node is created
dynamically and its fwnode device (fwnode->dev) is set to the PCI root
bridge device using the fw_devlink_set_device(&np->fwnode, &bridge->dev)
call.

On removal, of_pci_remove_host_bridge_node() is called and calls
device_remove_of_node() which in turn set to NULL the related
dev->fwnode.

Later in the removal sequence, device_del() is called and runs its
cleanup logic:

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

Because dev->fwnode has been cleared earlier, fw_devlink_set_device()
is not called and leaves fwnode->dev unchanged. This fwnode device
(fwnode->dev) becomes an dangling pointer.

If any reference to the OF node is held after this removal, the pointer
is still accessible using the OF node (np->fwnode.dev) but points to a
freed area.

The exact same issue is present in the of_pci_make_host_bridge_node()
error path leading to the exact same dangling fwnode->dev.

Avoid this dangling fwnode->dev pointer by clearing it in
of_pci_remove_host_bridge_node() and in of_pci_make_host_bridge_node()
error path.

Fixes: 1f340724419e ("PCI: of: Create device tree PCI host bridge node")
Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index ee9eb384b377..6a533f9bbf3c 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -743,6 +743,7 @@ void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge)
 	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
 		return;
 
+	fw_devlink_set_device(&np->fwnode, NULL);
 	device_remove_of_node(&bridge->bus->dev);
 	device_remove_of_node(&bridge->dev);
 	of_changeset_revert(np->data);
@@ -808,7 +809,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
 
 	ret = of_changeset_apply(cset);
 	if (ret)
-		goto out_free_node;
+		goto out_clear_devlink_dev;
 
 	np->data = cset;
 
@@ -830,6 +831,8 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
 out_revert_cset:
 	np->data = NULL;
 	of_changeset_revert(cset);
+out_clear_devlink_dev:
+	fw_devlink_set_device(&np->fwnode, NULL);
 out_free_node:
 	of_node_put(np);
 out_destroy_cset:
-- 
2.55.0

Re: [PATCH v10 07/10] PCI: of: Clear fwnode->dev during root bridge node removal and error path

[sashiko-bot]

> During the of_pci_make_host_bridge_node() call, an OF node is created
> dynamically and its fwnode device (fwnode->dev) is set to the PCI root
> bridge device using the fw_devlink_set_device(&np->fwnode, &bridge->dev)
> call.
> 
> On removal, of_pci_remove_host_bridge_node() is called and calls
> device_remove_of_node() which in turn set to NULL the related
> dev->fwnode.
> 
> Later in the removal sequence, device_del() is called and runs its
> cleanup logic:
> 
>     if (dev->fwnode && dev->fwnode->dev == dev)
>         fw_devlink_set_device(dev->fwnode, NULL);
> 
> [ ... ]
> error path.
> 
> Fixes: 1f340724419e ("PCI: of: Create device tree PCI host bridge node")
> Cc: stable@vger.kernel.org
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

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

[Herve Codina]

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.

When later an overlay is applied, this confuses fw_devlink. Indeed,
without any device attached to the node, fw_devlink considers that this
node will never become a device. When this node is pointed as a
supplier, devlink looks at its ancestors in order to find a node with a
device that could be used as the supplier.

In the PCI use case, this leads to links that wrongly use the PCI root
bridge device as the supplier instead of the expected PCI device.

Setting the fwnode device to the device of the PCI device allows devlink
to use this device as a supplier and so, correct links are created.

Fixes: 407d1a51921e ("PCI: Create device tree node for bridge")
Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/of.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 6a533f9bbf3c..a98f360c289a 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);
 	of_changeset_revert(np->data);
 	of_changeset_destroy(np->data);
@@ -709,9 +710,16 @@ 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);
+
 	ret = of_changeset_apply(cset);
 	if (ret)
-		goto out_free_node;
+		goto out_clear_devlink_dev;
 
 	np->data = cset;
 
@@ -726,6 +734,8 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
 out_revert_cset:
 	np->data = NULL;
 	of_changeset_revert(cset);
+out_clear_devlink_dev:
+	fw_devlink_set_device(&np->fwnode, NULL);
 out_free_node:
 	of_node_put(np);
 out_destroy_cset:
-- 
2.55.0

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

[sashiko-bot]

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 <herve.codina@bootlin.com>

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/20260717145147.823749-1-herve.codina@bootlin.com?part=8

[PATCH v10 09/10] PCI: of: Remove fwnode_dev_initialized() call for a PCI root bridge node

[Herve Codina]

During the instantiation of devices described by a device-tree overlay
applied on a PCI device, devlink displays the following kind of debug
messages instead of creating the expected links:
   'Not linking xxxx - might never become dev'

Without those expected links, the device removal order cannot be
correct.

Those debug traces are printed by fw_devlink_create_devlink(). In our
use case, they are all printed because the supplier of the link has at
least one of its ancestor with its fwnode flag FWNODE_FLAG_INITIALIZED
set.

The culprit ancestor is the PCI root bridge.

The fwnode related to the PCI root bridge is created dynamically by the
of_pci_make_host_bridge_node() function. During this creation
fwnode_dev_initialized() is called which set the FWNODE_FLAG_INITIALIZED
flag.

Calling fwnode_dev_initialized() tells devlink that the device related
to this node is handled out of the driver core. This is not correct in
our case. Indeed the device related to this firmware node is handled
using driver core mechanisms and is fully compliant devlink
expectations.

Simply remove the fwnode_dev_initialized() call. With that done, the
devlink debug messages are no more displayed and links that were missing
are correctly created.

Fixes: 1f340724419e ("PCI: of: Create device tree PCI host bridge node")
Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/of.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index a98f360c289a..28896f748e8d 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -815,7 +815,6 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
 	 */
 	of_node_set_flag(np, OF_POPULATED);
 	fw_devlink_set_device(&np->fwnode, &bridge->dev);
-	fwnode_dev_initialized(&np->fwnode, true);
 
 	ret = of_changeset_apply(cset);
 	if (ret)
-- 
2.55.0

Re: [PATCH v10 09/10] PCI: of: Remove fwnode_dev_initialized() call for a PCI root bridge node

[sashiko-bot]

> During the instantiation of devices described by a device-tree overlay
> applied on a PCI device, devlink displays the following kind of debug
> messages instead of creating the expected links:
>    'Not linking xxxx - might never become dev'
> 
> Without those expected links, the device removal order cannot be
> correct.
> 
> Those debug traces are printed by fw_devlink_create_devlink(). In our
> use case, they are all printed because the supplier of the link has at
> least one of its ancestor with its fwnode flag FWNODE_FLAG_INITIALIZED
> set.
> 
> The culprit ancestor is the PCI root bridge.
> 
> [ ... ]
> are correctly created.
> 
> Fixes: 1f340724419e ("PCI: of: Create device tree PCI host bridge node")
> Cc: stable@vger.kernel.org
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCH v10 10/10] PCI: of: Guard against node removal with incorrect np->data

[Herve Codina]

of_pci_remove_node() and of_pci_remove_host_bridge_node() check
whether the node is dynamic but not whether it has valid private data.

During the node creation, an OF changeset is used and this changeset is
stored in np->data to be available for removal functions.

If, for instance, a PCI host bridge is created using a device-tree
overlay, the related node will have the dynamic flag set but np->data
will be NULL. This leads to NULL pointer dereferences.

Further more, having a NULL np->data pointer means that the node has
been created out of our PCI node creation process and so shouldn't be
handled by our PCI node removal process.

Add a np->data check for NULL to bail out early of PCI node removal
functions.

Fixes: 407d1a51921e ("PCI: Create device tree node for bridge")
Fixes: 1f340724419e ("PCI: of: Create device tree PCI host bridge node")
Cc: <stable+noautosel@kernel.org> # Not triggered but 'This could be a problem...'
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 28896f748e8d..9f35e1e07cdc 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -654,7 +654,7 @@ void of_pci_remove_node(struct pci_dev *pdev)
 	struct device_node *np;
 
 	np = pci_device_to_OF_node(pdev);
-	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
+	if (!np || !of_node_check_flag(np, OF_DYNAMIC) || !np->data)
 		return;
 
 	fw_devlink_set_device(&np->fwnode, NULL);
@@ -750,7 +750,7 @@ void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge)
 	struct device_node *np;
 
 	np = pci_bus_to_OF_node(bridge->bus);
-	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
+	if (!np || !of_node_check_flag(np, OF_DYNAMIC) || !np->data)
 		return;
 
 	fw_devlink_set_device(&np->fwnode, NULL);
-- 
2.55.0

Re: [PATCH v10 10/10] PCI: of: Guard against node removal with incorrect np->data

[sashiko-bot]

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

Pre-existing issues:
- [High] Unsafe cast of `np->data` to `struct of_changeset *` without type validation, leading to type confusion and potential memory corruption.
--

--- Patch [10]: [PATCH v10 10/10] PCI: of: Guard against node removal with incorrect np->data ---
commit 3c2e9ae91c2c8b16b99779200dfab12179834af1
Author: Herve Codina <herve.codina@bootlin.com>

PCI: of: Guard against node removal with incorrect np->data

This commit adds a check for !np->data in of_pci_remove_node() and
of_pci_remove_host_bridge_node() to prevent NULL pointer dereferences. It
protects against dynamic nodes created by device-tree overlays that do not
populate np->data.

> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 28896f748e8d3..9f35e1e07cdcd 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -654,7 +654,7 @@ void of_pci_remove_node(struct pci_dev *pdev)
>  	struct device_node *np;
>  
>  	np = pci_device_to_OF_node(pdev);
> -	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
> +	if (!np || !of_node_check_flag(np, OF_DYNAMIC) || !np->data)
>  		return;
>  
>  	fw_devlink_set_device(&np->fwnode, NULL);

[Severity: High]
This isn't a bug introduced by this patch, but does this still leave a type
confusion issue if np->data is populated by another subsystem?

While this patch prevents a NULL pointer dereference, it doesn't verify that a
non-NULL np->data actually points to a struct of_changeset. On some platforms
like PowerPC, the OF_RECONFIG_ATTACH_NODE notifier (e.g., in
pci_dn_reconfig_notifier()) intercepts node additions and populates np->data
with its own structure, such as a struct pci_dn.

If that happens, and the device is later removed, of_pci_remove_node() will
see a non-NULL np->data, proceed past this check, and pass the foreign pointer
to of_changeset_revert() and of_changeset_destroy() lower down in the function:

	fw_devlink_set_device(&np->fwnode, NULL);
	device_remove_of_node(&pdev->dev);
	of_changeset_revert(np->data);
	of_changeset_destroy(np->data);

Could this result in memory corruption or a kernel panic when the wrong type
is cast and dereferenced?

[ ... ]

> @@ -750,7 +750,7 @@ void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge)
>  	struct device_node *np;
>  
>  	np = pci_bus_to_OF_node(bridge->bus);
> -	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
> +	if (!np || !of_node_check_flag(np, OF_DYNAMIC) || !np->data)
>  		return;
>  
>  	fw_devlink_set_device(&np->fwnode, NULL);

[Severity: High]
This isn't a bug introduced by this patch, but the same type confusion issue
seems to apply here in of_pci_remove_host_bridge_node(). 

If the host bridge node's np->data is populated by another subsystem with a
different structure, will passing it to of_changeset_revert() and
of_changeset_destroy() cause memory corruption?

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