Re: [PATCH 06/22] arm64: dts: qcom: monaco: Reserve low IOVA range for Iris

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

Pre-existing issues:
- [High] Pre-existing flaws in `of_iommu_get_resv_regions` cause a kernel panic on translation failure, break parsing for multiple device entries, and leak device node references.
--

--- Patch [6]: [PATCH 06/22] arm64: dts: qcom: monaco: Reserve low IOVA range for Iris ---
commit 80dd45ef37e8a085b1612e1aef1e011d1e74726c
Author: Vikash Garodia <[email protected]>

arm64: dts: qcom: monaco: Reserve low IOVA range for Iris

Iris VPU reserves IOVA addresses below 0x25800000 (600MB), primarily for
secure non-pixel buffers. Add a reserved-memory IOVA reservation node
covering [0, 0x25800000] and reference it from the Iris node.

> diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
> index cfe39f923224f..048b4490fcafd 100644
> --- a/arch/arm64/boot/dts/qcom/monaco.dtsi
> +++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
[ ... ]
> @@ -812,6 +812,17 @@ video_mem: video-region@9be82000 {
>  			reg = <0x0 0x9be82000 0x0 0x700000>;
>  			no-map;
>  		};
> +
> +		/*
> +		 * The Iris VPU reserves IOVA below 0x25800000 (600MB),
> +		 * primarily for non-pixel buffers using different Stream IDs.
> +		 * DMA into that range triggers unhandled SMMU faults and
> +		 * spontaneous reboots, so reserve it to keep IOMMU
> +		 * allocations above this boundary.
> +		 */
> +		iris_iova: iris-iova {
> +			iommu-addresses = <&iris 0x0 0x0 0x0 0x25800000>;

[Severity: High]
This is a pre-existing issue, but does using the iommu-addresses property
expose flaws in the underlying parser in of_iommu_get_resv_regions()?

Looking at drivers/iommu/of_iommu.c:of_iommu_get_resv_regions():

		while (maps < end) {
			struct device_node *np;
			u32 phandle;

			phandle = be32_to_cpup(maps++);
			np = of_find_node_by_phandle(phandle);

			if (np == dev->of_node) {
				...
				maps = of_translate_dma_region(np, maps, &iova, &length);

If of_translate_dma_region() fails, it returns NULL. The maps pointer becomes
NULL, and the while loop condition (maps < end) continues evaluating to true.
The next iteration calls be32_to_cpup(maps++), which dereferences NULL and
panics the kernel.

Additionally, if the parsed phandle does not match dev->of_node, maps is not
advanced past the skipped device's address and size cells. This causes the next
iteration to incorrectly read part of the DMA address as a phandle, which breaks
parsing for any property with multiple entries.

Finally, np is acquired via of_find_node_by_phandle() which increments the
refcount, but of_node_put() is never called to release it. Does this leak the
device node reference?

While this patch adds only a single-entry array and might bypass the multi-entry
parsing failure, the parser itself appears vulnerable to panicking on malformed
device trees.

> +		};
>  	};
>  
>  	smp2p-adsp {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6