Re: [PATCH v2 1/2] 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] of_msi_xlate() prematurely aborts device tree hierarchy traversal when msi_np is NULL.
--

commit d227a3bc08eef0cbb22a6f227edca082ac29a9b8
Author: Fuad Tabba <fuad.tabba@linux.dev>
of/irq: Fix device node refcount leak in of_check_msi_parent()

This patch prevents a device node reference count leak in
of_check_msi_parent(). It ensures that the msi_node reference is
only kept when the incoming node pointer is NULL.

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 regression introduced by this patch, but could the traversal
loop in of_msi_xlate() prematurely abort when msi_np is NULL?

If a driver calls of_msi_xlate(dev, NULL, id) and the direct parent lacks an
msi-map property, of_map_msi_id() passes the NULL msi_np to of_map_id()
as filter_np:

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

Because filter_np is NULL, it returns 0 instead of -ENODEV.

This translates to !of_map_msi_id(...) evaluating to true in the loop in
of_msi_xlate(), hitting the break statement:

drivers/of/irq.c:of_msi_xlate() {
	...
	for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
		struct of_phandle_args msi_spec = {};

		if (!of_map_msi_id(parent_dev->of_node, id_in, msi_np, &msi_spec)) {
			if (msi_spec.np) {
				...
			}
			/* msi-map present but no match → stop walking */
			break;
		}
		...
	}
	...
}

Will this cause the function to incorrectly terminate the loop on the very
first node, conflating an absent msi-map with a present but unmatched
msi-map, and failing to find the MSI mapping in ancestor nodes?

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