Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] drm/rockchip: DMABUF_DEBUG crash on fallback iommu_map_sgtable import
--

commit 59e8ac2bc0441a5d0b8e3d0e4089c7508adc71d5
Author: Karl Mehltretter <[email protected]>

drm/rockchip: Map imported buffers from DMA addresses

This commit updates rockchip_gem_iommu_map() to use iommu_map_sgtable_dma()
for imported buffers to avoid CONFIG_DMABUF_DEBUG import failures. If the
DMA mapping is not supported, it falls back to the old page-based path using
iommu_map_sgtable().

Link: https://lore.kernel.org/r/[email protected]/
Link: https://lore.kernel.org/r/[email protected]/

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> index 9a1dc9f120729..501c94cb921ad 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c

[ ... ]

> @@ -40,8 +41,16 @@ static int rockchip_gem_iommu_map(struct 
> rockchip_gem_object *rk_obj)
>  
>       rk_obj->dma_addr = rk_obj->mm.start;
>  
> -     ret = iommu_map_sgtable(private->domain, rk_obj->dma_addr, rk_obj->sgt,
> -                             prot);
> +     if (dma_dev) {
> +             ret = iommu_map_sgtable_dma(private->domain, rk_obj->dma_addr,
> +                                         dma_dev, rk_obj->sgt, prot);
> +             if (ret == -EOPNOTSUPP)
> +                     ret = iommu_map_sgtable(private->domain, 
> rk_obj->dma_addr,
> +                                             rk_obj->sgt, prot);

[Severity: Medium]
Will this fallback path crash if CONFIG_DMABUF_DEBUG is enabled?

The commit message mentions that unsupported imports still use the old
page-based path and remain unfixed. When iommu_map_sgtable_dma() returns
-EOPNOTSUPP, this code falls back to iommu_map_sgtable() which reads CPU-side
pages. Since the dma-buf core explicitly clears these CPU-side page pointers
when CONFIG_DMABUF_DEBUG is enabled, won't this fallback lead to an invalid
memory access or kernel panic?

Should the code return an error instead of falling back to a function that
is known to crash under these conditions?

> +     } else {
> +             ret = iommu_map_sgtable(private->domain, rk_obj->dma_addr,
> +                                     rk_obj->sgt, prot);
> +     }
>       if (ret < (ssize_t)rk_obj->base.size) {
>               DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n",

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to