[PATCH v2 0/2] of/irq: msi-parent handling in of_msi_xlate()

[PATCH v2 0/2] of/irq: msi-parent handling in of_msi_xlate()

[Fuad Tabba]

Hi folks,

Changes since v1 [1]:
  - Added patch 2: end the walk at the first node that declares an
    msi-parent, with the reference handling moved into of_msi_xlate().
    (Lorenzo)
  - Patch 1 is unchanged.

Patch 1 is the v1 fix: of_check_msi_parent() hands back a node reference
even when the caller passed one in as a filter, which of_msi_xlate()
documents as receiving none. Patch 2 is what Lorenzo asked for [2]: a
node declaring an msi-parent names the MSI controller for what sits
below it, so the walk ends there rather than carrying on to an
ancestor's msi-map.

Patch 2 removes the line patch 1 adds, along with the rest of the
reference handling. They stay separate because patch 1 is a small fix
for a leak that has been there since v6.18, and patch 2 changes
behaviour: folding them would hide the fix inside the change.

Based on Linux 7.3-rc1 (cee9395acd804).

Cheers,
/fuad

[1] https://lore.kernel.org/all/20260904073424.3855365-1-fuad.tabba@linux.dev/
[2] https://lore.kernel.org/all/apqnaNmeuUjfC8Ng@red-moon/

Fuad Tabba (2):
  of/irq: Fix device node refcount leak in of_check_msi_parent()
  of/irq: Stop the MSI walk at the first msi-parent

 drivers/of/irq.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

-- 
2.39.5

[PATCH v2 1/2] of/irq: Fix device node refcount leak in of_check_msi_parent()

[Fuad Tabba]

of_msi_xlate() documents @msi_np as a filter when it points at a node on
entry, handing back a reference only when it points at NULL. The msi-map
branch does that. of_check_msi_parent() keeps the reference from the 1:1
msi-parent match either way.

pci_msi_domain_get_msi_rid() is the only caller that passes a node in,
from irq_domain_get_of_node(), and it puts nothing, so a 1:1 match there
leaks the MSI controller node.

Fixes: 119aaeed0b67 ("of/irq: Add msi-parent check to of_msi_xlate()")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260826113604.010C01F000E9@smtp.kernel.org/
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/of/irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index ec035367c9500..109c54f346479 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -785,7 +785,7 @@ static int of_check_msi_parent(struct device_node *dev_node, struct device_node
 	if ((*msi_node && *msi_node != msi_spec.np) || msi_spec.args_count != 0)
 		ret = -EINVAL;
 
-	if (!ret) {
+	if (!ret && !*msi_node) {
 		/* Return with a node reference held */
 		*msi_node = msi_spec.np;
 		return 0;
-- 
2.39.5

[PATCH v2 2/2] of/irq: Stop the MSI walk at the first msi-parent

[Fuad Tabba]

of_check_msi_parent() returns -EINVAL both when the msi-parent names a
controller other than the one the caller filters on and when #msi-cells
isn't 0, so of_msi_xlate() can't tell the two apart and carries on up
the hierarchy. An ancestor's msi-map then maps the device onto a
controller its own node didn't name.

Hand the parsed specifier back to of_msi_xlate(), which owns the
reference as it already does on the msi-map path, and end the walk at
any node that declares an msi-parent.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260904074800.875391F00A3F@smtp.kernel.org/
Suggested-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
Link: https://lore.kernel.org/all/apqnaNmeuUjfC8Ng@red-moon/
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/of/irq.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 109c54f346479..4fbbd13f4d0a8 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -765,34 +765,27 @@ void __init of_irq_init(const struct of_device_id *matches)
 	}
 }
 
-static int of_check_msi_parent(struct device_node *dev_node, struct device_node **msi_node)
+static int of_check_msi_parent(struct device_node *dev_node, struct device_node **msi_node,
+			       struct of_phandle_args *msi_spec)
 {
-	struct of_phandle_args msi_spec;
 	int ret;
 
 	/*
 	 * An msi-parent phandle with a missing or == 0 #msi-cells
 	 * property identifies a 1:1 ID translation mapping.
 	 *
-	 * Set the msi controller node if the firmware matches this
-	 * condition.
+	 * @msi_spec keeps a reference to the target node whenever the
+	 * phandle parses, -EINVAL included, and the caller releases it.
 	 */
 	ret = of_parse_phandle_with_optional_args(dev_node, "msi-parent", "#msi-cells",
-						  0, &msi_spec);
+						  0, msi_spec);
 	if (ret)
 		return ret;
 
-	if ((*msi_node && *msi_node != msi_spec.np) || msi_spec.args_count != 0)
-		ret = -EINVAL;
+	if ((*msi_node && *msi_node != msi_spec->np) || msi_spec->args_count != 0)
+		return -EINVAL;
 
-	if (!ret && !*msi_node) {
-		/* Return with a node reference held */
-		*msi_node = msi_spec.np;
-		return 0;
-	}
-	of_node_put(msi_spec.np);
-
-	return ret;
+	return 0;
 }
 
 /**
@@ -806,7 +799,9 @@ static int of_check_msi_parent(struct device_node *dev_node, struct device_node
  * @id_in: Device ID.
  *
  * Walk up the device hierarchy looking for devices with a "msi-map"
- * or "msi-parent" property. If found, apply the mapping to @id_in.
+ * or "msi-parent" property. If found, apply the mapping to @id_in. With
+ * @msi_np non-NULL, a device declaring an msi-parent ends the walk, usable
+ * or not.
  *
  * Returns: The mapped MSI id.
  */
@@ -821,6 +816,7 @@ u32 of_msi_xlate(struct device *dev, struct device_node **msi_np, u32 id_in)
 	 */
 	for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
 		struct of_phandle_args msi_spec = {};
+		int ret;
 
 		if (!of_map_msi_id(parent_dev->of_node, id_in, msi_np, &msi_spec)) {
 			if (msi_spec.np) {
@@ -835,8 +831,17 @@ u32 of_msi_xlate(struct device *dev, struct device_node **msi_np, u32 id_in)
 			break;
 		}
 		/* -ENODEV: msi-map absent → check for msi-parent */
-		if (msi_np && !of_check_msi_parent(parent_dev->of_node, msi_np))
+		if (!msi_np)
+			continue;
+
+		ret = of_check_msi_parent(parent_dev->of_node, msi_np, &msi_spec);
+		if (msi_spec.np) {
+			/* A declared msi-parent names the controller, usable or not */
+			if (!ret && !*msi_np)
+				*msi_np = of_node_get(msi_spec.np);
+			of_node_put(msi_spec.np);
 			break;
+		}
 	}
 	return id_out;
 }
-- 
2.39.5