Re: [PATCH v2] drm/amdgpu: reject mapping info when BO VA is gone

[Yousef Alhouseen]

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"
<christian.koenig@xxxxxxx> 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
> > <alhouseenyousef@xxxxxxxxx> 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 <christian.koenig@xxxxxxx>
> >> Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> >> ---
> >> 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