Re: [PATCH 13/22] arm64: dts: qcom: kodiak: Reserve low IOVA range for Venus

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

Pre-existing issues:
- [High] Fatal parsing bug and NULL pointer dereference in `of_iommu_get_resv_regions` when parsing `iommu-addresses`.
--

--- Patch [13]: [PATCH 13/22] arm64: dts: qcom: kodiak: Reserve low IOVA range for Venus ---
commit c5c55dfe3f391bbec75ef2147bce3c7b4d05e83b
Author: Vikash Garodia <[email protected]>
Subject: arm64: dts: qcom: kodiak: Reserve low IOVA range for Venus

This commit reserves IOVA addresses below 0x25800000 for the Venus VPU to
prevent SMMU page faults and spontaneous reboots caused by unhandled DMA.
It adds a reserved-memory IOVA reservation node and references it from the
venus node to keep IOMMU allocations above this boundary.

> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
[ ... ]
> @@ -198,6 +198,17 @@ adsp_rpc_remote_heap_mem: adsp-rpc-remote-heap {
>  			alignment = <0x0 0x400000>;
>  			size = <0x0 0x800000>;
>  		};
> +
> +		/*
> +		 * The Venus 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.
> +		 */
> +		venus_iova: venus-iova {
> +			iommu-addresses = <&venus 0x0 0x0 0x0 0x25800000>;

[Severity: High]
This isn't a bug introduced by this patch, but adding this iommu-addresses
property exposes a fatal parsing flaw in the IOMMU core layer that can panic
the kernel.

In drivers/iommu/of_iommu.c:of_iommu_get_resv_regions(), the loop parsing
the iommu-addresses property assigns the map pointer without validation:

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

If of_translate_dma_region() fails, it returns NULL. The code does not
check for this, meaning maps becomes NULL. The loop condition (maps < end)
then evaluates to true, and the next iteration will directly dereference
NULL.

Additionally, if the parsed phandle does not match dev->of_node:

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

The code falls through without advancing the maps pointer to skip the
current device's address and size cells. This causes the next iteration to
incorrectly read address cells as phandles, entirely corrupting the DT
parsing structure.

Is there any risk that this new configuration could trigger a translation
failure or phandle mismatch during device probe and hit these bugs?

> +		};
>  	};
>  
>  	cpus {

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