Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

drm/amdgpu/userq: refcount userqueues to avoid any race conditions

To avoid race condition and avoid UAF cases, implement kref
based queues and protect the below operations using xa lock
a. Getting a queue from xarray
b. Increment/Decrement it's refcount

Every time some one want to access a queue, always get via
amdgpu_userq_get to make sure we have locks in place and get
the object if active.

A userqueue is destroyed on the last refcount is dropped which
typically would be via IOCTL or during fini.

v2: Add the missing drop in one the condition in the signal ioclt [Alex]

v3: remove the queue from the xarray first in the free queue ioctl path
[Christian]

- Pass queue to the amdgpu_userq_put directly.
- make amdgpu_userq_put xa_lock free since we are doing put for each get
only and final put is done via destroy and we remove the queue from xa
with lock.
- use userq_put in fini too so cleanup is done fully.

v4: Use xa_erase directly rather than doing load and erase in free
ioctl. Also remove some of the error logs which could be exploited
by the user to flood the logs [Christian]

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

authored by

Sunil Khatri and committed by
Alex Deucher
4952189b c7c57327

+95 -39
+81 -35
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
··· 446 446 return ret; 447 447 } 448 448 449 - static void amdgpu_userq_cleanup(struct amdgpu_usermode_queue *queue, 450 - u32 queue_id) 449 + static void amdgpu_userq_cleanup(struct amdgpu_usermode_queue *queue) 451 450 { 452 451 struct amdgpu_userq_mgr *uq_mgr = queue->userq_mgr; 453 452 struct amdgpu_device *adev = uq_mgr->adev; ··· 460 461 uq_funcs->mqd_destroy(queue); 461 462 amdgpu_userq_fence_driver_free(queue); 462 463 /* Use interrupt-safe locking since IRQ handlers may access these XArrays */ 463 - xa_erase_irq(&uq_mgr->userq_xa, queue_id); 464 464 xa_erase_irq(&adev->userq_doorbell_xa, queue->doorbell_index); 465 465 queue->userq_mgr = NULL; 466 466 list_del(&queue->userq_va_list); 467 467 kfree(queue); 468 468 469 469 up_read(&adev->reset_domain->sem); 470 - } 471 - 472 - static struct amdgpu_usermode_queue * 473 - amdgpu_userq_find(struct amdgpu_userq_mgr *uq_mgr, u32 qid) 474 - { 475 - return xa_load(&uq_mgr->userq_xa, qid); 476 470 } 477 471 478 472 void ··· 617 625 } 618 626 619 627 static int 620 - amdgpu_userq_destroy(struct drm_file *filp, u32 queue_id) 628 + amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_queue *queue) 621 629 { 622 - struct amdgpu_fpriv *fpriv = filp->driver_priv; 623 - struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr; 624 630 struct amdgpu_device *adev = uq_mgr->adev; 625 - struct amdgpu_usermode_queue *queue; 626 631 int r = 0; 627 632 628 633 cancel_delayed_work_sync(&uq_mgr->resume_work); 629 634 mutex_lock(&uq_mgr->userq_mutex); 630 - queue = amdgpu_userq_find(uq_mgr, queue_id); 631 - if (!queue) { 632 - drm_dbg_driver(adev_to_drm(uq_mgr->adev), "Invalid queue id to destroy\n"); 633 - mutex_unlock(&uq_mgr->userq_mutex); 634 - return -EINVAL; 635 - } 636 635 amdgpu_userq_wait_for_last_fence(queue); 637 636 /* Cancel any pending hang detection work and cleanup */ 638 637 if (queue->hang_detect_fence) { ··· 655 672 drm_warn(adev_to_drm(uq_mgr->adev), "trying to destroy a HW mapping userq\n"); 656 673 queue->state = AMDGPU_USERQ_STATE_HUNG; 657 674 } 658 - amdgpu_userq_cleanup(queue, queue_id); 675 + amdgpu_userq_cleanup(queue); 659 676 mutex_unlock(&uq_mgr->userq_mutex); 660 677 661 678 pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); 662 679 663 680 return r; 681 + } 682 + 683 + static void amdgpu_userq_kref_destroy(struct kref *kref) 684 + { 685 + int r; 686 + struct amdgpu_usermode_queue *queue = 687 + container_of(kref, struct amdgpu_usermode_queue, refcount); 688 + struct amdgpu_userq_mgr *uq_mgr = queue->userq_mgr; 689 + 690 + r = amdgpu_userq_destroy(uq_mgr, queue); 691 + if (r) 692 + drm_file_err(uq_mgr->file, "Failed to destroy usermode queue %d\n", r); 693 + } 694 + 695 + struct amdgpu_usermode_queue *amdgpu_userq_get(struct amdgpu_userq_mgr *uq_mgr, u32 qid) 696 + { 697 + struct amdgpu_usermode_queue *queue; 698 + 699 + xa_lock(&uq_mgr->userq_xa); 700 + queue = xa_load(&uq_mgr->userq_xa, qid); 701 + if (queue) 702 + kref_get(&queue->refcount); 703 + xa_unlock(&uq_mgr->userq_xa); 704 + 705 + return queue; 706 + } 707 + 708 + void amdgpu_userq_put(struct amdgpu_usermode_queue *queue) 709 + { 710 + if (queue) 711 + kref_put(&queue->refcount, amdgpu_userq_kref_destroy); 664 712 } 665 713 666 714 static int amdgpu_userq_priority_permit(struct drm_file *filp, ··· 848 834 goto unlock; 849 835 } 850 836 837 + /* drop this refcount during queue destroy */ 838 + kref_init(&queue->refcount); 839 + 851 840 /* Wait for mode-1 reset to complete */ 852 841 down_read(&adev->reset_domain->sem); 853 842 r = xa_err(xa_store_irq(&adev->userq_doorbell_xa, index, queue, GFP_KERNEL)); ··· 1002 985 struct drm_file *filp) 1003 986 { 1004 987 union drm_amdgpu_userq *args = data; 1005 - int r; 988 + struct amdgpu_fpriv *fpriv = filp->driver_priv; 989 + struct amdgpu_usermode_queue *queue; 990 + int r = 0; 1006 991 1007 992 if (!amdgpu_userq_enabled(dev)) 1008 993 return -ENOTSUPP; ··· 1019 1000 drm_file_err(filp, "Failed to create usermode queue\n"); 1020 1001 break; 1021 1002 1022 - case AMDGPU_USERQ_OP_FREE: 1023 - r = amdgpu_userq_destroy(filp, args->in.queue_id); 1024 - if (r) 1025 - drm_file_err(filp, "Failed to destroy usermode queue\n"); 1003 + case AMDGPU_USERQ_OP_FREE: { 1004 + xa_lock(&fpriv->userq_mgr.userq_xa); 1005 + queue = __xa_erase(&fpriv->userq_mgr.userq_xa, args->in.queue_id); 1006 + xa_unlock(&fpriv->userq_mgr.userq_xa); 1007 + if (!queue) 1008 + return -ENOENT; 1009 + 1010 + amdgpu_userq_put(queue); 1026 1011 break; 1012 + } 1027 1013 1028 1014 default: 1029 1015 drm_dbg_driver(dev, "Invalid user queue op specified: %d\n", args->in.op); ··· 1047 1023 1048 1024 /* Resume all the queues for this process */ 1049 1025 xa_for_each(&uq_mgr->userq_xa, queue_id, queue) { 1026 + queue = amdgpu_userq_get(uq_mgr, queue_id); 1027 + if (!queue) 1028 + continue; 1029 + 1050 1030 if (!amdgpu_userq_buffer_vas_mapped(queue)) { 1051 1031 drm_file_err(uq_mgr->file, 1052 1032 "trying restore queue without va mapping\n"); 1053 1033 queue->state = AMDGPU_USERQ_STATE_INVALID_VA; 1034 + amdgpu_userq_put(queue); 1054 1035 continue; 1055 1036 } 1056 1037 1057 1038 r = amdgpu_userq_restore_helper(queue); 1058 1039 if (r) 1059 1040 ret = r; 1041 + 1042 + amdgpu_userq_put(queue); 1060 1043 } 1061 1044 1062 1045 if (ret) ··· 1297 1266 amdgpu_userq_detect_and_reset_queues(uq_mgr); 1298 1267 /* Try to unmap all the queues in this process ctx */ 1299 1268 xa_for_each(&uq_mgr->userq_xa, queue_id, queue) { 1269 + queue = amdgpu_userq_get(uq_mgr, queue_id); 1270 + if (!queue) 1271 + continue; 1300 1272 r = amdgpu_userq_preempt_helper(queue); 1301 1273 if (r) 1302 1274 ret = r; 1275 + amdgpu_userq_put(queue); 1303 1276 } 1304 1277 1305 1278 if (ret) ··· 1336 1301 int ret; 1337 1302 1338 1303 xa_for_each(&uq_mgr->userq_xa, queue_id, queue) { 1304 + queue = amdgpu_userq_get(uq_mgr, queue_id); 1305 + if (!queue) 1306 + continue; 1307 + 1339 1308 struct dma_fence *f = queue->last_fence; 1340 1309 1341 - if (!f || dma_fence_is_signaled(f)) 1310 + if (!f || dma_fence_is_signaled(f)) { 1311 + amdgpu_userq_put(queue); 1342 1312 continue; 1313 + } 1343 1314 ret = dma_fence_wait_timeout(f, true, msecs_to_jiffies(100)); 1344 1315 if (ret <= 0) { 1345 1316 drm_file_err(uq_mgr->file, "Timed out waiting for fence=%llu:%llu\n", 1346 1317 f->context, f->seqno); 1318 + amdgpu_userq_put(queue); 1347 1319 return -ETIMEDOUT; 1348 1320 } 1321 + amdgpu_userq_put(queue); 1349 1322 } 1350 1323 1351 1324 return 0; ··· 1404 1361 void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr) 1405 1362 { 1406 1363 struct amdgpu_usermode_queue *queue; 1407 - unsigned long queue_id; 1364 + unsigned long queue_id = 0; 1408 1365 1409 - cancel_delayed_work_sync(&userq_mgr->resume_work); 1366 + for (;;) { 1367 + xa_lock(&userq_mgr->userq_xa); 1368 + queue = xa_find(&userq_mgr->userq_xa, &queue_id, ULONG_MAX, 1369 + XA_PRESENT); 1370 + if (queue) 1371 + __xa_erase(&userq_mgr->userq_xa, queue_id); 1372 + xa_unlock(&userq_mgr->userq_xa); 1410 1373 1411 - mutex_lock(&userq_mgr->userq_mutex); 1412 - amdgpu_userq_detect_and_reset_queues(userq_mgr); 1413 - xa_for_each(&userq_mgr->userq_xa, queue_id, queue) { 1414 - amdgpu_userq_wait_for_last_fence(queue); 1415 - amdgpu_userq_unmap_helper(queue); 1416 - amdgpu_userq_cleanup(queue, queue_id); 1374 + if (!queue) 1375 + break; 1376 + 1377 + amdgpu_userq_put(queue); 1417 1378 } 1418 1379 1419 1380 xa_destroy(&userq_mgr->userq_xa); 1420 - mutex_unlock(&userq_mgr->userq_mutex); 1421 1381 mutex_destroy(&userq_mgr->userq_mutex); 1422 1382 } 1423 1383
+4
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
··· 74 74 struct dentry *debugfs_queue; 75 75 struct delayed_work hang_detect_work; 76 76 struct dma_fence *hang_detect_fence; 77 + struct kref refcount; 77 78 78 79 struct list_head userq_va_list; 79 80 }; ··· 114 113 uint32_t doorbell_offset; 115 114 struct amdgpu_userq_obj *db_obj; 116 115 }; 116 + 117 + struct amdgpu_usermode_queue *amdgpu_userq_get(struct amdgpu_userq_mgr *uq_mgr, u32 qid); 118 + void amdgpu_userq_put(struct amdgpu_usermode_queue *queue); 117 119 118 120 int amdgpu_userq_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); 119 121
+10 -4
drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
··· 469 469 struct drm_gem_object **gobj_write, **gobj_read; 470 470 u32 *syncobj_handles, num_syncobj_handles; 471 471 struct amdgpu_userq_fence *userq_fence; 472 - struct amdgpu_usermode_queue *queue; 472 + struct amdgpu_usermode_queue *queue = NULL; 473 473 struct drm_syncobj **syncobj = NULL; 474 474 struct dma_fence *fence; 475 475 struct drm_exec exec; ··· 519 519 goto put_gobj_read; 520 520 521 521 /* Retrieve the user queue */ 522 - queue = xa_load(&userq_mgr->userq_xa, args->queue_id); 522 + queue = amdgpu_userq_get(userq_mgr, args->queue_id); 523 523 if (!queue) { 524 524 r = -ENOENT; 525 525 goto put_gobj_write; ··· 610 610 free_syncobj_handles: 611 611 kfree(syncobj_handles); 612 612 613 + if (queue) 614 + amdgpu_userq_put(queue); 615 + 613 616 return r; 614 617 } 615 618 ··· 627 624 struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr; 628 625 struct drm_gem_object **gobj_write, **gobj_read; 629 626 u32 *timeline_points, *timeline_handles; 630 - struct amdgpu_usermode_queue *waitq; 627 + struct amdgpu_usermode_queue *waitq = NULL; 631 628 u32 *syncobj_handles, num_syncobj; 632 629 struct dma_fence **fences = NULL; 633 630 u16 num_points, num_fences = 0; ··· 863 860 */ 864 861 num_fences = dma_fence_dedup_array(fences, num_fences); 865 862 866 - waitq = xa_load(&userq_mgr->userq_xa, wait_info->waitq_id); 863 + waitq = amdgpu_userq_get(userq_mgr, wait_info->waitq_id); 867 864 if (!waitq) { 868 865 r = -EINVAL; 869 866 goto free_fences; ··· 946 943 kfree(timeline_handles); 947 944 free_syncobj_handles: 948 945 kfree(syncobj_handles); 946 + 947 + if (waitq) 948 + amdgpu_userq_put(waitq); 949 949 950 950 return r; 951 951 }