Re: [PATCH] drm/amdgpu: reject mapping info for unmapped BOs

["Christian König" <christian.koenig@amd.com>]

On 6/25/26 11:07, Yousef Alhouseen wrote:
> Hi Christian,
> 
> You're right. I rechecked the handle-open path and bo_va should
> already be created for this file, so I do not have a valid reproducer
> for the NULL case.
> 
> Please drop this patch.

Well wait a second, I think you stumbled over something here. It's just that your bug description is not correct.

As far as I can see it is possible that between drm_gem_object_lookup() and drm_exec_lock_obj() the handle will be closed. So in this case bo_va will then be NULL.

It's a rather small race window, that's why I asked if you can reproduce this because we would then add this case to our IGT tests.

But if you can't reproduce this then well it should still be fixed. Just update the commit message and maybe return -EINVAL instead of -ENOENT.

Regards,
Christian.

> 
> Thanks,
> Yousef
> 
> On Thu, 25 Jun 2026 10:46:24 +0200, "Christian König"
> <christian.koenig@amd.com> wrote:
>> On 6/24/26 19:20, Yousef Alhouseen wrote:
>>> AMDGPU_GEM_OP_GET_MAPPING_INFO looks up the BO's VM mapping and then
>>> iterates the valid and invalid mapping lists unconditionally. A GEM BO can
>>> be queried before it has been mapped into the file VM, in which case
>>> amdgpu_vm_bo_find() returns NULL and the list walk dereferences it.
>>
>> Mhm, that is not correct at all.
>>
>> The bo_va is created when the handle is opened inside the filp and not when the first mapping is created.
>>
>> Do you have a test case to reproduce the issue?
>>
>> Thanks,
>> Christian.
>>
>>>
>>> Return -ENOENT for an unmapped BO, matching the VA operation path that
>>> already rejects missing BO-VA state before touching the mapping lists.
>>>
>>> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
>>> ---
>>> 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..4b2699931 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 = -ENOENT;
>>> + 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
>>>