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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>

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@xxxxxxx>

Christian.

                 return;
         }

--
2.25.1


_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux