Fix out-of-bounds issue in userq fence create when accessing the userq xa structure. Added a lock to protect the race condition. v2:(Christian) - Acquire xa lock only for the xa_for_each blocks and not for the kvmalloc_array() memory allocation part. v3: - Replacing the kvmalloc_array() storage with xa_alloc() solves the problem. BUG: KASAN: slab-out-of-bounds in amdgpu_userq_fence_create+0x726/0x880 [amdgpu] [ +0.000006] Call Trace: [ +0.000005] <TASK> [ +0.000005] dump_stack_lvl+0x6c/0x90 [ +0.000011] print_report+0xc4/0x5e0 [ +0.000009] ? srso_return_thunk+0x5/0x5f [ +0.000008] ? kasan_complete_mode_report_info+0x26/0x1d0 [ +0.000007] ? amdgpu_userq_fence_create+0x726/0x880 [amdgpu] [ +0.000405] kasan_report+0xdf/0x120 [ +0.000009] ? amdgpu_userq_fence_create+0x726/0x880 [amdgpu] [ +0.000405] __asan_report_store8_noabort+0x17/0x20 [ +0.000007] amdgpu_userq_fence_create+0x726/0x880 [amdgpu] [ +0.000406] ? __pfx_amdgpu_userq_fence_create+0x10/0x10 [amdgpu] [ +0.000408] ? srso_return_thunk+0x5/0x5f [ +0.000008] ? ttm_resource_move_to_lru_tail+0x235/0x4f0 [ttm] [ +0.000013] ? srso_return_thunk+0x5/0x5f [ +0.000008] amdgpu_userq_signal_ioctl+0xd29/0x1c70 [amdgpu] [ +0.000412] ? __pfx_amdgpu_userq_signal_ioctl+0x10/0x10 [amdgpu] [ +0.000404] ? try_to_wake_up+0x165/0x1840 [ +0.000010] ? __pfx_futex_wake_mark+0x10/0x10 [ +0.000011] drm_ioctl_kernel+0x178/0x2f0 [drm] [ +0.000050] ? __pfx_amdgpu_userq_signal_ioctl+0x10/0x10 [amdgpu] [ +0.000404] ? __pfx_drm_ioctl_kernel+0x10/0x10 [drm] [ +0.000043] ? __kasan_check_read+0x11/0x20 [ +0.000007] ? srso_return_thunk+0x5/0x5f [ +0.000007] ? __kasan_check_write+0x14/0x20 [ +0.000008] drm_ioctl+0x513/0xd20 [drm] [ +0.000040] ? __pfx_amdgpu_userq_signal_ioctl+0x10/0x10 [amdgpu] [ +0.000407] ? __pfx_drm_ioctl+0x10/0x10 [drm] [ +0.000044] ? srso_return_thunk+0x5/0x5f [ +0.000007] ? _raw_spin_lock_irqsave+0x99/0x100 [ +0.000007] ? __pfx__raw_spin_lock_irqsave+0x10/0x10 [ +0.000006] ? __rseq_handle_notify_resume+0x188/0xc30 [ +0.000008] ? srso_return_thunk+0x5/0x5f [ +0.000008] ? srso_return_thunk+0x5/0x5f [ +0.000006] ? _raw_spin_unlock_irqrestore+0x27/0x50 [ +0.000010] amdgpu_drm_ioctl+0xcd/0x1d0 [amdgpu] [ +0.000388] __x64_sys_ioctl+0x135/0x1b0 [ +0.000009] x64_sys_call+0x1205/0x20d0 [ +0.000007] do_syscall_64+0x4d/0x120 [ +0.000008] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ +0.000007] RIP: 0033:0x7f7c3d31a94f Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@xxxxxxx> --- .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 43 +++++++------------ .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 3 +- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c index 2e7271362ace..4289bed6c1f7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -122,10 +122,11 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv) { + struct amdgpu_userq_fence_driver *stored_fence_drv; struct amdgpu_userq_fence *userq_fence, *tmp; struct dma_fence *fence; + unsigned long index; u64 rptr; - int i; if (!fence_drv) return; @@ -141,8 +142,8 @@ void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_d dma_fence_signal(fence); - for (i = 0; i < userq_fence->fence_drv_array_count; i++) - amdgpu_userq_fence_driver_put(userq_fence->fence_drv_array[i]); + xa_for_each(&userq_fence->fence_drv_xa, index, stored_fence_drv) + amdgpu_userq_fence_driver_put(stored_fence_drv); list_del(&userq_fence->link); dma_fence_put(fence); @@ -221,34 +222,24 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq, dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock, fence_drv->context, seq); + xa_init_flags(&userq_fence->fence_drv_xa, XA_FLAGS_ALLOC); + amdgpu_userq_fence_driver_get(fence_drv); dma_fence_get(fence); if (!xa_empty(&userq->fence_drv_xa)) { struct amdgpu_userq_fence_driver *stored_fence_drv; - unsigned long index, count = 0; - int i = 0; - - xa_for_each(&userq->fence_drv_xa, index, stored_fence_drv) - count++; - - userq_fence->fence_drv_array = - kvmalloc_array(count, - sizeof(struct amdgpu_userq_fence_driver *), - GFP_KERNEL); - - if (userq_fence->fence_drv_array) { - xa_for_each(&userq->fence_drv_xa, index, stored_fence_drv) { - userq_fence->fence_drv_array[i] = stored_fence_drv; - xa_erase(&userq->fence_drv_xa, index); - i++; - } + unsigned long index_uq; + u32 index_uf; + int err; + + xa_for_each(&userq->fence_drv_xa, index_uq, stored_fence_drv) { + err = xa_alloc_irq(&userq_fence->fence_drv_xa, &index_uf, + stored_fence_drv, xa_limit_32b, GFP_KERNEL); + if (err) + return err; } - - userq_fence->fence_drv_array_count = i; - } else { - userq_fence->fence_drv_array = NULL; - userq_fence->fence_drv_array_count = 0; + xa_destroy(&userq->fence_drv_xa); } /* Check if hardware has already processed the job */ @@ -300,8 +291,6 @@ static void amdgpu_userq_fence_free(struct rcu_head *rcu) /* Release the fence driver reference */ amdgpu_userq_fence_driver_put(fence_drv); - - kvfree(userq_fence->fence_drv_array); kmem_cache_free(amdgpu_userq_fence_slab, userq_fence); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h index f1a90840ac1f..3283e5573d10 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h @@ -37,9 +37,8 @@ struct amdgpu_userq_fence { */ spinlock_t lock; struct list_head link; - unsigned long fence_drv_array_count; + struct xarray fence_drv_xa; struct amdgpu_userq_fence_driver *fence_drv; - struct amdgpu_userq_fence_driver **fence_drv_array; }; struct amdgpu_userq_fence_driver { -- 2.25.1