Re: [RFC PATCH 2/2] drm/rockchip: Map imported buffers from DMA addresses

[sashiko-bot@kernel.org]

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 <kmehltretter@gmail.com>

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/DLR74W1U9YPC.375IK0HOYHDIG@cknow-tech.com/
Link: https://lore.kernel.org/r/DLYLA3WQNN3X.3FB9MO8ZWBQ5@cknow-tech.com/

> 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/20261009002114.67851-1-kmehltretter@gmail.com?part=2