Re: [PATCH 04/11] drm/amdgpu: rework amdgpu_userq_signal_ioctl

["Khatri, Sunil" <sukhatri@amd.com>]

On 23-04-2026 04:17 pm, Christian König wrote:
> Hi guys,
>
> On 4/23/26 11:58, Liang, Prike wrote:
> ...
>>> -static int amdgpu_userq_fence_alloc(struct amdgpu_userq_fence **userq_fence)
>>> +static int amdgpu_userq_fence_alloc(struct amdgpu_usermode_queue *userq,
>>> +                                 struct amdgpu_userq_fence **pfence)
>>>   {
>>> -     *userq_fence = kmalloc(sizeof(**userq_fence), GFP_ATOMIC);
>>> -     return *userq_fence ? 0 : -ENOMEM;
>>> +     struct amdgpu_userq_fence_driver *fence_drv = userq->fence_drv;
>>> +     struct amdgpu_userq_fence *userq_fence;
>>> +     unsigned long count;
>> We must initialize count; otherwise, it may contain a garbage value, which can cause amdgpu_userq_fence_alloc() to fail and,
>> in turn, make userq fence emission fail.
> I've got the same comment from both Sunil and Prike but as far as I can see  and that is actually incorrect.
>
>>> +     userq_fence = kmalloc(sizeof(*userq_fence), GFP_KERNEL);
>>> +     if (!userq_fence)
>>> +             return -ENOMEM;
>>> +
>>> +     /*
>>> +      * Get the next unused entry, since we fill from the start this can be
>>> +      * used as size to allocate the array.
>>> +      */
>>> +     mutex_lock(&userq->fence_drv_lock);
>>> +     xa_find(&userq->fence_drv_xa, &count, ULONG_MAX, XA_FREE_MARK);
> The count should be initialized here. But could be that this doesn't work.
>
> Did you guys got a KASAN warning or something like that?
I got application crash recieving -ENOMEM. The reason that count isnt 
initialized and as per information i gather xa_find need the starting 
index i.e pointed by count else it will start with whatever garbage 
value it have.

Regards
Sunil Khatri
>
>>> +
>>> +     userq_fence->fence_drv_array = kvmalloc_array(count, sizeof(fence_drv),
>>> +                                                   GFP_KERNEL);
>>> +     if (!userq_fence->fence_drv_array) {
>>> +             mutex_unlock(&userq->fence_drv_lock);
>>> +             kfree(userq_fence);
>>> +             return -ENOMEM;
>>> +     }
>>> +
>>> +     userq_fence->fence_drv_array_count = count;
>>> +     xa_extract(&userq->fence_drv_xa, (void **)userq_fence->fence_drv_array,
>>> +                0, ULONG_MAX, count, XA_PRESENT);
>> We may need to assign the userq_fence->fence_drv_array_count the exact copied number from the xa_extract().
> Interresting point. Why could that differ ?
>
> Thanks for the comments,
> Christian.