* [PATCH] drm/amdgpu: Fix out-of-bounds read when update mapping
@ 2021-07-27 10:37 Pan, Xinhui
2021-07-28 11:14 ` Christian König
0 siblings, 1 reply; 2+ messages in thread
From: Pan, Xinhui @ 2021-07-27 10:37 UTC (permalink / raw)
To: amd-gfx; +Cc: Deucher, Alexander, Koenig, Christian
[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@amd.com>
---
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);
return;
}
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/amdgpu: Fix out-of-bounds read when update mapping
2021-07-27 10:37 [PATCH] drm/amdgpu: Fix out-of-bounds read when update mapping Pan, Xinhui
@ 2021-07-28 11:14 ` Christian König
0 siblings, 0 replies; 2+ messages in thread
From: Christian König @ 2021-07-28 11:14 UTC (permalink / raw)
To: Pan, Xinhui, amd-gfx; +Cc: Deucher, Alexander
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@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@amd.com>
Christian.
> return;
> }
>
> --
> 2.25.1
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-07-28 11:14 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 10:37 [PATCH] drm/amdgpu: Fix out-of-bounds read when update mapping Pan, Xinhui
2021-07-28 11:14 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.