From bf9cd275752b935e9c968c12656182ee476a0027 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Wed, 8 May 2024 22:30:54 +0000 Subject: [PATCH] turnip: virtio: fix iova leak upon found already imported dmabuf There's a success path on found dmabuf while the iova won't be cleaned up. This change defers iova alloc till lookup miss and also to prepare for later racy dmabuf re-import fix. Also documented a potential leak on error path due to unable to tell whether a gem handle should be closed or not without refcounting. Fixes: f17c5297d7a ("tu: Add virtgpu support") Signed-off-by: Yiwei Zhang Part-of: (cherry picked from commit 6ca192f586fe1470a97b6f34bce7761a760ce15d) --- .pick_status.json | 2 +- src/freedreno/vulkan/tu_knl_drm_virtio.cc | 28 ++++++++++++----------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index e217e742889..07f686adafb 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -664,7 +664,7 @@ "description": "turnip: virtio: fix iova leak upon found already imported dmabuf", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "f17c5297d7a01eb37815f96bbf3a87667a2f3261", "notes": null diff --git a/src/freedreno/vulkan/tu_knl_drm_virtio.cc b/src/freedreno/vulkan/tu_knl_drm_virtio.cc index 2257ad3a025..040dfd8f22b 100644 --- a/src/freedreno/vulkan/tu_knl_drm_virtio.cc +++ b/src/freedreno/vulkan/tu_knl_drm_virtio.cc @@ -675,11 +675,6 @@ virtio_bo_init_dmabuf(struct tu_device *dev, /* iova allocation needs to consider the object's *real* size: */ size = real_size; - uint64_t iova; - result = virtio_allocate_userspace_iova(dev, size, 0, TU_BO_ALLOC_NO_FLAGS, &iova); - if (result != VK_SUCCESS) - return result; - /* Importing the same dmabuf several times would yield the same * gem_handle. Thus there could be a race when destroying * BO and importing the same dmabuf from different threads. @@ -689,6 +684,7 @@ virtio_bo_init_dmabuf(struct tu_device *dev, u_rwlock_wrlock(&dev->dma_bo_lock); uint32_t handle, res_id; + uint64_t iova; handle = vdrm_dmabuf_to_handle(vdrm, prime_fd); if (!handle) { @@ -698,6 +694,7 @@ virtio_bo_init_dmabuf(struct tu_device *dev, res_id = vdrm_handle_to_res_id(vdrm, handle); if (!res_id) { + /* XXX gem_handle potentially leaked here since no refcnt */ result = vk_error(dev, VK_ERROR_INVALID_EXTERNAL_HANDLE); goto out_unlock; } @@ -714,21 +711,26 @@ virtio_bo_init_dmabuf(struct tu_device *dev, bo->res_id = res_id; - result = tu_bo_init(dev, bo, handle, size, iova, - TU_BO_ALLOC_NO_FLAGS, "dmabuf"); - if (result != VK_SUCCESS) - memset(bo, 0, sizeof(*bo)); - else - *out_bo = bo; + result = virtio_allocate_userspace_iova(dev, size, 0, TU_BO_ALLOC_NO_FLAGS, + &iova); + if (result != VK_SUCCESS) { + vdrm_bo_close(dev->vdev->vdrm, handle); + goto out_unlock; + } -out_unlock: - u_rwlock_wrunlock(&dev->dma_bo_lock); + result = + tu_bo_init(dev, bo, handle, size, iova, TU_BO_ALLOC_NO_FLAGS, "dmabuf"); if (result != VK_SUCCESS) { mtx_lock(&dev->vma_mutex); util_vma_heap_free(&dev->vma, iova, size); mtx_unlock(&dev->vma_mutex); + memset(bo, 0, sizeof(*bo)); + } else { + *out_bo = bo; } +out_unlock: + u_rwlock_wrunlock(&dev->dma_bo_lock); return result; }