amdgpu_ualink_export_handle() tests robj->ualink_handle_lo and stores
the generated handle into it without any lock; xa_lock only covers
handle generation and the xarray insert.
Two threads can therefore both observe a zero handle for the same BO and
both allocate an exporter node, take an amdgpu_bo_ref() and insert into
exp_xa, with the last store to ualink_handle_lo winning. Teardown only
erases the handle recorded in the BO, so the losing node keeps its BO
reference and pinned VRAM until device teardown.
Each racing thread still generates its own handle and inserts its own
node, but the handle is published to the BO under the BO reservation
lock after re-checking that ualink_handle_lo is still zero. Exactly one
thread makes that transition; the others find a non-zero value, roll
back their node (erase from exp_xa, drop the DMABuf and BO references,
free it) and return -EAGAIN so the caller retries and picks up the
winning handle. This keeps a single exporter node per BO.
Fixes: 6b1a5ad90778 ("drm/amdgpu: Implement UALink handle export")
Reported-by: Sashiko-bot <[email protected]>
Signed-off-by: Mukul Joshi <[email protected]>
---
v1->v2:
- Use amdgpu_bo_reserve() to update the ualink_handle_lo in the BO.
If it is not zero, then roll back the changes.
drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 31 +++++++++++++++++-----
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 6e937f31a5b1..39b7cc8bc1c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -4023,9 +4023,7 @@ int amdgpu_ualink_export_handle(struct drm_device *dev,
struct drm_file *filp,
if (IS_ERR(exp_xa_node->dmabuf)) {
r = PTR_ERR(exp_xa_node->dmabuf);
dev_err(adev->dev, "Failed to generate DMABuf for the
BO\n");
- amdgpu_bo_unref(&exp_xa_node->bo);
- kfree(exp_xa_node);
- goto out;
+ goto err_unref_bo;
}
xa_lock(&adev->ualink.exp_xa);
@@ -4036,13 +4034,22 @@ int amdgpu_ualink_export_handle(struct drm_device *dev,
struct drm_file *filp,
xa_unlock(&adev->ualink.exp_xa);
if (r) {
dev_err(adev->dev, "Failed to insert exp_xa_node into
XA: %d\n", r);
- dma_buf_put(exp_xa_node->dmabuf);
- amdgpu_bo_unref(&exp_xa_node->bo);
- kfree(exp_xa_node);
- goto out;
+ goto err_put_dmabuf;
+ }
+
+ r = amdgpu_bo_reserve(robj, false);
+ if (r)
+ goto err_erase_xa;
+
+ if (robj->ualink_handle_lo) {
+ amdgpu_bo_unreserve(robj);
+ r = -EAGAIN;
+ goto err_erase_xa;
}
robj->ualink_handle_lo = handle.handle_lo;
+ amdgpu_bo_unreserve(robj);
+
/* Return the generated handle back to the caller */
*handle_out = handle;
} else {
@@ -4059,6 +4066,16 @@ int amdgpu_ualink_export_handle(struct drm_device *dev,
struct drm_file *filp,
handle_out->handle_hi = exp_xa_node->handle.handle_hi;
}
+ drm_gem_object_put(gobj);
+ return 0;
+
+err_erase_xa:
+ xa_erase(&adev->ualink.exp_xa, handle.handle_lo);
+err_put_dmabuf:
+ dma_buf_put(exp_xa_node->dmabuf);
+err_unref_bo:
+ amdgpu_bo_unref(&exp_xa_node->bo);
+ kfree(exp_xa_node);
out:
drm_gem_object_put(gobj);
return r;
--
2.55.0
