No, you are not blind. I generated v2 from a local branch that still contained v1, so the v2 diff added a second check on top of the intended one. I then mistook that local state for the target tree when withdrawing it.
The close-handle race you identified still calls for the single check, but neither of my postings is suitable as sent. I will prepare a clean v3 against the target branch. Sorry for the confusion. Thanks, Yousef On Mon, 29 Jun 2026 14:33:37 +0200, "Christian König" <[email protected]> wrote: > On 6/25/26 16:26, Yousef Alhouseen wrote: > > Hi Alex, Christian, > > > > Please drop this v2 as well. > > > > I rechecked the target tree and missed the existing !bo_va check in > > AMDGPU_GEM_OP_GET_MAPPING_INFO. > > Am I blind? As far as I can see at least the code in drm-misc-next doesn't > have such a check. > > Regards, > Christian. > > > The close-race case is already handled > > before the mapping list walks, so this patch only makes the later check > > unreachable and changes the errno. > > > > Sorry for the churn. > > > > Thanks, > > Yousef > > > > On Thu, 25 Jun 2026 15:53:41 +0200, Yousef Alhouseen > > <[email protected]> wrote: > >> AMDGPU_GEM_OP_GET_MAPPING_INFO looks up the GEM object from the file > >> handle and then locks the object and VM before resolving the BO-VA. The > >> GEM object reference keeps the BO alive, but it does not keep the > >> per-file handle open. > >> > >> If a racing close drops the last handle reference in that window, > >> amdgpu_gem_object_close() can remove the BO-VA before > >> amdgpu_vm_bo_find() runs. The ioctl then walks the BO-VA mapping lists > >> unconditionally. > >> > >> Return -EINVAL if the BO is no longer associated with this VM. > >> > >> Suggested-by: Christian König <[email protected]> > >> Signed-off-by: Yousef Alhouseen <[email protected]> > >> --- > >> Changes in v2: > >> - Describe the handle-close race instead of an initially unmapped BO. > >> - Return -EINVAL instead of -ENOENT. > >> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >> index 212c14d99..6f5b6f4c2 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >> @@ -1087,6 +1087,12 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, > >> void *data, > >> struct drm_amdgpu_gem_vm_entry *vm_entries; > >> struct amdgpu_bo_va_mapping *mapping; > >> int num_mappings = 0; > >> + > >> + if (!bo_va) { > >> + r = -EINVAL; > >> + goto out_exec; > >> + } > >> + > >> /* > >> * num_entries is set as an input to the size of the user-allocated array of > >> * drm_amdgpu_gem_vm_entry stored at args->value. > >> -- > >> 2.54.0
