[PATCH] drm/amdgpu: Fix out-of-bounds read when update mapping

[Christian König]

Am 27.07.21 um 12:37 schrieb Pan, Xinhui:
> [AMD Official Use Only]
>
> If one GTT BO has been evicted/swapped out, it should sit in CPU domain.
> TTM only alloc struct ttm_resource instead of struct ttm_range_mgr_node
> for sysMem.
>
> Now when we update mapping for such invalidated BOs, we might walk out
> of bounds of struct ttm_resource.
>
> Three possible fix:
> 1) Let sysMem manager alloc struct ttm_range_mgr_node, like
> ttm_range_manager does.
> 2) Pass pages_addr to update_mapping function too, but need memset
> pages_addr[] to zero when unpopulate.
> 3) Init amdgpu_res_cursor directly.
>
>
> bug is detected by kfence.
> ==================================================================
> BUG: KFENCE: out-of-bounds read in amdgpu_vm_bo_update_mapping+0x564/0x6e0
>
> Out-of-bounds read at 0x000000008ea93fe9 (64B right of kfence-#167):
>   amdgpu_vm_bo_update_mapping+0x564/0x6e0 [amdgpu]
>   amdgpu_vm_bo_update+0x282/0xa40 [amdgpu]
>   amdgpu_vm_handle_moved+0x19e/0x1f0 [amdgpu]
>   amdgpu_cs_vm_handling+0x4e4/0x640 [amdgpu]
>   amdgpu_cs_ioctl+0x19e7/0x23c0 [amdgpu]
>   drm_ioctl_kernel+0xf3/0x180 [drm]
>   drm_ioctl+0x2cb/0x550 [drm]
>   amdgpu_drm_ioctl+0x5e/0xb0 [amdgpu]
>
> kfence-#167 [0x000000008e11c055-0x000000001f676b3e
>   ttm_sys_man_alloc+0x35/0x80 [ttm]
>   ttm_resource_alloc+0x39/0x50 [ttm]
>   ttm_bo_swapout+0x252/0x5a0 [ttm]
>   ttm_device_swapout+0x107/0x180 [ttm]
>   ttm_global_swapout+0x6f/0x130 [ttm]
>   ttm_tt_populate+0xb1/0x2a0 [ttm]
>   ttm_bo_handle_move_mem+0x17e/0x1d0 [ttm]
>   ttm_mem_evict_first+0x59d/0x9c0 [ttm]
>   ttm_bo_mem_space+0x39f/0x400 [ttm]
>   ttm_bo_validate+0x13c/0x340 [ttm]
>   ttm_bo_init_reserved+0x269/0x540 [ttm]
>   amdgpu_bo_create+0x1d1/0xa30 [amdgpu]
>   amdgpu_bo_create_user+0x40/0x80 [amdgpu]
>   amdgpu_gem_object_create+0x71/0xc0 [amdgpu]
>   amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu+0x2f2/0xcd0 [amdgpu]
>   kfd_ioctl_alloc_memory_of_gpu+0xe2/0x330 [amdgpu]
>   kfd_ioctl+0x461/0x690 [amdgpu]
>
> Signed-off-by: xinhui pan <xinhui.pan at amd.com>

Good catch.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> index 59e0fefb15aa..4d8058121678 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> @@ -54,11 +54,12 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
>   {
>          struct drm_mm_node *node;
>
> -       if (!res) {
> +       if (!res || res->mem_type == TTM_PL_SYSTEM) {
>                  cur->start = start;
>                  cur->size = size;
>                  cur->remaining = size;
>                  cur->node = NULL;
> +               BUG_ON(res && start + size > res->num_pages << PAGE_SHIFT);

Either drop that or make it just a WARN_ON(), BUG_ON() is certainly to 
hard here.

With that fixed that patch is Reviewed-by: Christian König 
<christian.koenig at amd.com>

Christian.

>                  return;
>          }
>
> --
> 2.25.1
>