Re: [PATCH] drm/ttm: Fix ttm_bo_move_memcpy() for subclassed struct ttm_resource

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Aug 30, 2021 at 09:48:35AM +0200, Thomas Hellström wrote:
> The code was making a copy of a struct ttm_resource. However,
> recently the struct ttm_resources were allowed to be subclassed and
> also were allowed to be malloced, hence the driver could end up assuming
> the copy we handed it was subclassed and worse, the original could have
> been freed at this point.
> 
> Fix this by using the original struct ttm_resource before it is
> potentially freed in ttm_bo_move_sync_cleanup()
> 
> Reported-by: Ben Skeggs <skeggsb@gmail.com>
> Reported-by: Dave Airlie <airlied@gmail.com>
> Cc: Christian König <christian.koenig@amd.com>
> Fixes: 3bf3710e3718 ("drm/ttm: Add a generic TTM memcpy move for page-based iomem")
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

This doesn't apply cleanly to drm-misc-next-fixes, do we need a backmerge
or something? Can you pls coordinate with Maarten and then also poke for a
pull request so this isn't stuck?

Also since 5.14 is released this needs cc: stable.
-Daniel

> ---
>  drivers/gpu/drm/ttm/ttm_bo_util.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 5c20d0541cc3..c893c3db2623 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -139,7 +139,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>  	struct ttm_resource *src_mem = bo->resource;
>  	struct ttm_resource_manager *src_man =
>  		ttm_manager_type(bdev, src_mem->mem_type);
> -	struct ttm_resource src_copy = *src_mem;
>  	union {
>  		struct ttm_kmap_iter_tt tt;
>  		struct ttm_kmap_iter_linear_io io;
> @@ -173,11 +172,10 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>  	if (!(clear && ttm && !(ttm->page_flags & TTM_PAGE_FLAG_ZERO_ALLOC)))
>  		ttm_move_memcpy(clear, dst_mem->num_pages, dst_iter, src_iter);
>  
> -	src_copy = *src_mem;
> +	if (!src_iter->ops->maps_tt)
> +		ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, src_mem);
>  	ttm_bo_move_sync_cleanup(bo, dst_mem);
>  
> -	if (!src_iter->ops->maps_tt)
> -		ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, &src_copy);
>  out_src_iter:
>  	if (!dst_iter->ops->maps_tt)
>  		ttm_kmap_iter_linear_io_fini(&_dst_iter.io, bdev, dst_mem);
> -- 
> 2.31.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch