Re: [PATCH 08/22] arm64: dts: qcom: sm8350: 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] Uninitialized variable usage and NULL pointer dereference in of_iommu_get_resv_regions() when of_translate_dma_region() fails.
--

--- Patch [8]: [PATCH 08/22] arm64: dts: qcom: sm8350: Reserve low IOVA range for Iris ---
commit ff9e2ea90c6d8293aa79fd7350941f4a90c4b7df
Author: Vikash Garodia <[email protected]>

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

The Iris VPU needs IOVA addresses below 0x25800000 reserved to prevent
unhandled SMMU page faults and reboots caused by DMA from other stream
IDs. This patch adds a reserved-memory IOVA reservation node and references
it from the Iris node.

Fixes: a2b0c5eba743 ("arm64: dts: qcom: sm8350: add Iris device")
Cc: [email protected]

> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 32b47f5ec1f67..b376dda0ed520 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi

[ ... ]

> @@ -553,6 +553,17 @@ removed_mem: memory@d8800000 {
>  			reg = <0x0 0xd8800000 0x0 0x6800000>;
>  			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 isn't a bug introduced by this patch, but adding this iommu-addresses
property highlights a pre-existing issue in the parsing code. Does
of_iommu_get_resv_regions() safely handle translation failures?

Looking at drivers/iommu/of_iommu.c:of_iommu_get_resv_regions(), when
of_translate_dma_region() fails, it returns NULL:

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

The return value is assigned directly to maps without checking for NULL,
leaving the iova and length stack variables uninitialized.

If the loop continues to the next iteration, it attempts to dereference
maps:

drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
    ...
    phandle = be32_to_cpup(maps++);
    ...
}

This causes a NULL pointer dereference and a kernel panic.

Could the parsing code in of_iommu.c be hardened to gracefully handle
invalid or untranslatable data without panicking?

> +		};
>  	};

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