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

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

Pre-existing issues:
- [High] Missing error handling for `of_translate_dma_region` leads to uninitialized stack usage and a NULL pointer dereference.
- [High] The loop parsing `iommu-addresses` fails to advance the property pointer when skipping entries for other devices, corrupting parsing.
--

--- Patch [11]: [PATCH 11/22] arm64: dts: qcom: sm8750: Reserve low IOVA range for Iris ---
commit c7d055796e0d5e48c39a3d7938283d3a8d0628e4
Author: Vikash Garodia <[email protected]>

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

This commit adds a reserved-memory IOVA reservation node for the Iris VPU to 
keep IOMMU allocations above 0x25800000, preventing unhandled SMMU faults.
It also references the reservation node from the Iris node.

> diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> index dd738d13df8e0..cf17c76a4a390 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> @@ -562,6 +562,17 @@ adsp_rpc_remote_heap_mem: adsp-rpc-remote-heap {
>  			size = <0x0 0xc00000>;
>  			reusable;
>  		};
> +
> +		/*
> +		 * 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 using iommu-addresses here
exposes a NULL pointer dereference risk in drivers/iommu/of_iommu.c if the
device tree is ever misconfigured.

If of_translate_dma_region() fails (e.g., due to invalid dma-ranges), it
returns NULL, which is assigned directly to maps without checking:

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

On the next loop iteration, maps is compared to end, and then dereferenced
via be32_to_cpup(maps++), causing a kernel oops and failing to boot:

drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
    ...
    while (maps < end) {
        struct device_node *np;
        u32 phandle;

        phandle = be32_to_cpup(maps++);
    ...
}

In addition, length and iova remain uninitialized stack variables. Can we
add a check for NULL after of_translate_dma_region() to prevent this crash?

[Severity: High]
This is a pre-existing issue, but adding iommu-addresses here is affected by
a parsing bug in drivers/iommu/of_iommu.c for multi-entry properties.

When parsing the array, if the phandle does not match dev->of_node, the code
skips the if block and loops, but fails to advance past the DMA address and
size cells for the skipped entry:

drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
    ...
        if (np == dev->of_node) {
            int err;

            maps = of_translate_dma_region(np, maps, &iova, &length);
    ...
}

The next loop iteration will misinterpret the skipped DMA address as the next
phandle, corrupting the parsing entirely. Does this need to be fixed in
of_iommu_get_resv_regions() to safely support multi-entry iommu-addresses
properties?

[ ... ]

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