Re: [PATCH v2 2/2] arm64: dts: qcom: hamoa: Reserve low IOVA range for Iris

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing NULL check on the return value of `of_translate_dma_region()` causes use of uninitialized variables and a guaranteed NULL pointer dereference.
- [High] The parser fails to advance the `maps` pointer when skipping entries for other devices, leading to misaligned parsing and potential out-of-bounds memory reads.
--

commit 9504aa19b1b75ded98afc02b95aff54f7fd1ba37
Author: Daniel J Blueman <daniel@quora.org>

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

On X1-family hamoa platforms, the Iris VPU reserves IOVA addresses
below 0x25800000 (600MB), primarily for non-pixel buffers accessed
via different Stream IDs. DMA into that range triggers unhandled SMMU
page faults that cause spontaneous device reboots. This is readily
reproduced with one or more browser tabs driving multiple concurrent
video decode streams.

Link: https://github.com/qualcomm-linux/kernel-topics/issues/1157#issuecomment-4458933574

> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index 051dee0764167..ce96e7f8d8c10 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -716,6 +716,17 @@ smem_mem: smem@ffe00000 {
>  			hwlocks = <&tcsr_mutex 3>;
>  			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 exercises a pre-existing issue in
drivers/iommu/of_iommu.c:of_iommu_get_resv_regions().

If of_translate_dma_region() fails (e.g. returning OF_BAD_ADDR), it returns
NULL without populating the local variables iova and length. The maps
pointer is unconditionally overwritten with this NULL value:

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

The code then evaluates if (length == 0) using uninitialized stack garbage.

If it proceeds to the next iteration, the while (maps < end) condition
evaluates as true because NULL (0) is less than the valid kernel pointer
end. The next iteration then blindly dereferences the NULL pointer with
be32_to_cpup(maps++), causing a kernel panic.

Does the IOMMU core need to be fixed to handle a NULL return from
of_translate_dma_region() before adding new users of iommu-addresses?


[Severity: High]
This is a pre-existing issue, but there is a second bug in
drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() related to
iommu-addresses parsing that makes shared memory regions unsafe.

If an entry's phandle belongs to a different device, the loop skips the
translation block:

drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
    ...
    if (np == dev->of_node) {
        // ...
    }
    // missing else branch to advance maps pointer
    ...
}

Because it lacks an else branch to advance the maps pointer by the required
#address-cells + #size-cells, the next iteration erroneously reads the first
cell of the skipped address as a new phandle.

This desynchronizes the entire parser, potentially causing it to interpret
an address or size value as a matching device phandle. It would then call
of_translate_dma_region() on misaligned data, risking an out-of-bounds
memory read since there is no bounds checking against the end of the
property buffer.

Should the parsing logic be corrected to properly advance the maps pointer
for non-matching devices?

> +		};
>  	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614145113.84243-1-daniel@quora.org?part=2