On Mon, 30 Aug 2021 at 17:48, Thomas Hellström
<thomas.hellstrom@xxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> Reported-by: Dave Airlie <airlied@xxxxxxxxx>
> Cc: Christian König <christian.koenig@xxxxxxx>
> Fixes: 3bf3710e3718 ("drm/ttm: Add a generic TTM memcpy move for page-based iomem")
> Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
That's basically identical to what I came up with locally, so:
Reviewed-by: Ben Skeggs <bskeggs@xxxxxxxxxx>
> ---
> 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
>
