On Thu, Sep 24, 2026 at 10:31 AM Xiang Liu <[email protected]> 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]>
Acked-by: Alex Deucher <[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); > -- > 2.34.1 >
