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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] of_msi_xlate() fails to traverse the device hierarchy when msi_np is NULL, completely breaking MSI ID mapping for callers such as the fsl-mc subsystem.
- [High] msi_node reference is leaked on error path in its_v5_pci_msi_prepare()
- [High] of_msi_xlate() incorrectly handles 'msi-parent' by continuing tree traversal when it mismatches.
--

commit b8991c82a3994d7ebf04943226ad5d1d9490a8d4
Author: Fuad Tabba <fuad.tabba@linux.dev>

of/irq: Fix device node refcount leak in of_check_msi_parent()

The patch fixes a device node refcount leak in of_check_msi_parent(). It
ensures that if the node was not matched in a 1:1 mapping, the reference is
released correctly before returning.

Link: https://lore.kernel.org/all/20260826113604.010C01F000E9@smtp.kernel.org/

> 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;

[Severity: High]
This isn't a bug introduced by this patch, but of_msi_xlate() seems to
incorrectly handle msi-parent mismatches.

If a device specifies an msi-parent that does not match the msi_np filter,
of_check_msi_parent() returns -EINVAL.

drivers/of/irq.c:of_msi_xlate() {
		/* -ENODEV: msi-map absent -> check for msi-parent */
		if (msi_np && !of_check_msi_parent(parent_dev->of_node, msi_np))
			break;
}

The condition !of_check_msi_parent() evaluates to false, skipping the break
statement and allowing traversal to continue. Will this erroneously allow
traversal up the tree, potentially inheriting an incorrect msi-map from a
parent bus?

[Severity: High]
This is a pre-existing issue, but does of_msi_xlate() fail to traverse the
device hierarchy when msi_np is NULL?

When of_map_msi_id() (which calls of_map_id()) processes an absent map
property while filter_np is NULL, it returns 0 instead of an error:

drivers/of/base.c:of_map_id() {
	...
	map = of_get_property(np, map_name, &map_bytes);
	if (!map) {
		if (filter_np)
			return -ENODEV;
		/* Otherwise, no map implies no translation */
		arg->args[0] = id;
		arg->args_count = 1;
		return 0;
	}
	...
}

In of_msi_xlate(), this return value of 0 triggers a break:

drivers/of/irq.c:of_msi_xlate() {
		if (!of_map_msi_id(parent_dev->of_node, id_in, msi_np, &msi_spec)) {
			...
			/* msi-map present but no match -> stop walking */
			break;
		}
}

Because of_map_id() returns 0 when the map is absent, wouldn't this early
break erroneously abort the parent traversal loop on the first iteration,
breaking MSI ID mapping for callers passing a NULL msi_np?

[Severity: High]
This isn't a bug introduced by this patch, but is there a fwnode reference
leak on the error path in its_v5_pci_msi_prepare()?

drivers/irqchip/irq-gic-its-msi-parent.c:its_v5_pci_msi_prepare() {
	rid = pci_msi_map_rid_ctlr_node(domain->parent, pdev, &msi_node);
	if (!msi_node)
		return -ENODEV;

	ret = its_translate_frame_address(msi_node, &pa);
	if (ret)
		return -ENODEV;

	fwnode_handle_put(msi_node);
	...
}

If its_translate_frame_address() fails, it returns -ENODEV without dropping
the acquired msi_node reference. Can this leak the fwnode_handle if the
frame address translation fails repeatedly during driver bind or unbind?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904073424.3855365-1-fuad.tabba@linux.dev?part=1