On 2026-09-24 10:21, Xiang Liu wrote:
drm_gem_handle_create_tail() skips the close() callback when open()
returns an error, so a bo_va added before a failing step is never
removed. It stays linked on bo->vm_bo and a later eviction of that BO
dereferences the VM after it has been torn down.
Validate and fence the BO before adding the bo_va, drop the bo_va when
attaching the eviction fence fails, and propagate the allocation failure
from amdgpu_vm_bo_add().
Signed-off-by: Xiang Liu <[email protected]>
Reviewed-by: Felix Kuehling <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 80 ++++++++++++++-----------
1 file changed, 45 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9da1880ee76d..be0c642f3740 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -260,7 +260,7 @@ static int amdgpu_gem_object_open(struct drm_gem_object
*obj,
struct amdgpu_bo_va *bo_va;
struct mm_struct *mm;
struct drm_exec exec;
- int r;
+ int r = 0;
mm = amdgpu_ttm_tt_get_usermm(abo->tbo.ttm);
if (mm && mm != current->mm)
@@ -270,6 +270,42 @@ static int amdgpu_gem_object_open(struct drm_gem_object
*obj,
!amdgpu_vm_is_bo_always_valid(vm, abo))
return -EPERM;
+ /* Validate and add eviction fence to DMABuf imports with dynamic
+ * attachment in compute VMs. Re-validation will be done by
+ * amdgpu_vm_validate. Fences are on the reservation shared with the
+ * export, which is currently required to be validated and fenced
+ * already by amdgpu_amdkfd_gpuvm_restore_process_bos.
+ *
+ * This runs before the BO is added to the VM:
drm_gem_handle_create_tail()
+ * does not call the close() callback when open() fails, so nothing that
+ * would need unwinding may exist yet.
+ *
+ * Nested locking below for the case that a GEM object is opened in
+ * kfd_mem_export_dmabuf. Since the lock below is only taken for
imports,
+ * but not for export, this is a different lock class that cannot lead
to
+ * circular lock dependencies.
+ */
+ if (vm->is_compute_context && vm->process_info &&
+ drm_gem_is_imported(obj) &&
+ dma_buf_is_dynamic(obj->import_attach->dmabuf)) {
+ mutex_lock_nested(&vm->process_info->lock, 1);
+ if (!WARN_ON(!vm->process_info->eviction_fence))
+ r = amdgpu_amdkfd_bo_validate_and_fence(abo,
+ AMDGPU_GEM_DOMAIN_GTT,
+
&vm->process_info->eviction_fence->base);
+ mutex_unlock(&vm->process_info->lock);
+ if (r) {
+ struct amdgpu_task_info *ti =
amdgpu_vm_get_task_info_vm(vm);
+
+ dev_warn(adev->dev, "validate_and_fence failed: %d\n",
r);
+ if (ti) {
+ dev_warn(adev->dev, "pid %d\n", ti->task.pid);
+ amdgpu_vm_put_task_info(ti);
+ }
+ return r;
+ }
+ }
+
drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 0);
drm_exec_until_all_locked(&exec) {
r = drm_exec_prepare_obj(&exec, &abo->tbo.base,
TTM_NUM_MOVE_FENCES + 1);
@@ -287,47 +323,21 @@ static int amdgpu_gem_object_open(struct drm_gem_object
*obj,
bo_va = amdgpu_vm_bo_find(vm, abo);
if (!bo_va) {
bo_va = amdgpu_vm_bo_add(adev, vm, abo);
+ if (!bo_va) {
+ r = -ENOMEM;
+ goto out_unlock;
+ }
r = amdgpu_evf_mgr_attach_fence(&fpriv->evf_mgr, abo);
- if (r)
+ if (r) {
+ amdgpu_vm_bo_del(adev, bo_va);
goto out_unlock;
+ }
} else {
++bo_va->ref_count;
}
drm_exec_fini(&exec);
-
- /* Validate and add eviction fence to DMABuf imports with dynamic
- * attachment in compute VMs. Re-validation will be done by
- * amdgpu_vm_validate. Fences are on the reservation shared with the
- * export, which is currently required to be validated and fenced
- * already by amdgpu_amdkfd_gpuvm_restore_process_bos.
- *
- * Nested locking below for the case that a GEM object is opened in
- * kfd_mem_export_dmabuf. Since the lock below is only taken for
imports,
- * but not for export, this is a different lock class that cannot lead
to
- * circular lock dependencies.
- */
- if (!vm->is_compute_context || !vm->process_info)
- return 0;
- if (!drm_gem_is_imported(obj) ||
- !dma_buf_is_dynamic(obj->import_attach->dmabuf))
- return 0;
- mutex_lock_nested(&vm->process_info->lock, 1);
- if (!WARN_ON(!vm->process_info->eviction_fence)) {
- r = amdgpu_amdkfd_bo_validate_and_fence(abo,
AMDGPU_GEM_DOMAIN_GTT,
-
&vm->process_info->eviction_fence->base);
- if (r) {
- struct amdgpu_task_info *ti =
amdgpu_vm_get_task_info_vm(vm);
-
- dev_warn(adev->dev, "validate_and_fence failed: %d\n",
r);
- if (ti) {
- dev_warn(adev->dev, "pid %d\n", ti->task.pid);
- amdgpu_vm_put_task_info(ti);
- }
- }
- }
- mutex_unlock(&vm->process_info->lock);
- return r;
+ return 0;
out_unlock:
drm_exec_fini(&exec);
