Re: [PATCH v9 13/14] arm64: dts: qcom: glymur: Add iris video node

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The use of the `iommu-addresses` property triggers a severe preexisting parsing bug and memory leak in `of_iommu_get_resv_regions`.
--

--- Patch [13]: [PATCH v9 13/14] arm64: dts: qcom: glymur: Add iris video node ---
commit 973017069435379c05db5988e8245460d2e68365
Author: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

arm64: dts: qcom: glymur: Add iris video node

This commit adds the iris video codec to the glymur SoC. It introduces a
new reservation node and configures clocks, interconnects, interrupts, and
power domains for the video-codec.

> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
> index 55d91c696a3a3..0c09e700f6b59 100644
> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
> @@ -695,6 +695,10 @@ adsp_rpc_remote_heap_mem: adsp-rpc-remote-heap {
>  			alignment = <0x0 0x400000>;
>  			size = <0x0 0x800000>;
>  		};
> +
> +		iris_resv: reservation-iris {
> +			iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>;

[Severity: High]
This isn't a bug introduced by this patch, but using the iommu-addresses
property triggers severe parsing flaws and a memory leak in
of_iommu_get_resv_regions().

First, does the parser leak a device node reference on every loop iteration?
It acquires the node reference but fails to release it:

drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
    ...
    np = of_find_node_by_phandle(phandle);
    ...
}

Without a corresponding of_node_put(np) inside or after the loop, this leaks
memory.

Second, if the iommu-addresses array contains entries for multiple devices,
the loop fails to advance the maps pointer past the address and size cells
when np != dev->of_node. This causes subsequent iterations to erroneously
interpret IOVA address and size values as phandles.

Finally, could this code cause a kernel panic if translation fails?

drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
    ...
    maps = of_translate_dma_region(np, maps, &iova, &length);
    ...
}

If of_translate_dma_region() returns NULL, it is assigned to maps without
checking. This will lead to an immediate NULL pointer dereference in
be32_to_cpup(maps++) on the next loop iteration.

While this specific dtsi patch only provides a single entry and avoids the
incorrect cell shift, should these underlying driver bugs be addressed
upstream before adopting this property here?

> +		};
>  	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-glymur-v9-0-8cf2cbe12a07@oss.qualcomm.com?part=13