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

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the parsing loop in `of_iommu_get_resv_regions` fails to properly advance the `maps` pointer when a tuple's `phandle` does not match the target device, leading to corrupt parsing and potential out-of-bounds reads.
- [High] This problem wasn't introduced by this patch, but an unchecked return value from `of_translate_dma_region()` in `of_iommu_get_resv_regions()` leads to uninitialized stack variable usage and a NULL pointer dereference.
--

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

arm64: dts: qcom: talos: Reserve low IOVA range for Venus

This commit adds a reserved-memory IOVA reservation node for the Venus VPU
and references it to keep DMA allocations above 0x25800000. This prevents
unhandled SMMU page faults and spontaneous device reboots.

> diff --git a/arch/arm64/boot/dts/qcom/talos.dtsi b/arch/arm64/boot/dts/qcom/talos.dtsi
> index eedfdce3c479e..8645b04e26643 100644
> --- a/arch/arm64/boot/dts/qcom/talos.dtsi
> +++ b/arch/arm64/boot/dts/qcom/talos.dtsi
> @@ -687,6 +687,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 does adding an
iommu-addresses property expose an existing parsing issue in
of_iommu_get_resv_regions()?

When parsing the iommu-addresses property, if a tuple's phandle does not
match dev->of_node, the loop skips the match block but fails to properly
advance the maps pointer past the current tuple's address and size cells:

drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
    ...
    while (maps < end) {
        phandle = be32_to_cpup(maps++);
        np = of_find_node_by_phandle(phandle);
        
        if (np == dev->of_node) {
            ...
        }
        /* maps is not advanced by address/size cells if np != dev->of_node */
    }
    ...
}

Could this lead to the next iteration incorrectly treating part of the DMA
address as the next phandle, resulting in out-of-bounds reads or corrupt
parsing?

[Severity: High]
This is also a pre-existing issue, but could parsing this property lead to a
NULL pointer dereference if of_translate_dma_region() fails?

If of_translate_dma_region() returns NULL (e.g. due to missing dma-ranges
or a malformed DT), the return value is assigned directly to maps without
a NULL check:

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

Since length is evaluated without initialization if the translation fails,
and maps becomes NULL, could the subsequent loop iteration dereference the
NULL maps pointer when reading the next phandle?

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

> +		};
>  	};

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