Re: [PATCH] drm/amdgpu: validate SIGNAL/WAIT ioctl input argument

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

Oh, you inverted the check!

I haven't seen that, but that isn't necessary and actually messes things up a bit.

A NULL pointer is handled by the copy_from_user() functions and shouldn't be checked here because that results in incorrect return code.

So we clearly don't need that.

Regards,
Christian.

On 3/25/26 09:56, Liang, Prike wrote:
> [Public]
> 
> Yes, we still need Mesa to handle the userq sync handle count  (num_syncobj_handles = 0) allocation correctly before the kernel can safely filter out invalid *_number = 0 cases. The change below is aimed at filtering out invalid handle cases on the kernel side, which helps reject bogus handles without breaking the existing userq fence signal/wait IOCTL behavior, as I’ve tested.
> 
> Regards,
>       Prike
> 
>> -----Original Message-----
>> From: Koenig, Christian <Christian.Koenig@amd.com>
>> Sent: Tuesday, March 24, 2026 10:10 PM
>> To: Liang, Prike <Prike.Liang@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
>> Subject: Re: [PATCH] drm/amdgpu: validate SIGNAL/WAIT ioctl input argument
>>
>> I don't think we can do this right know. Userqueues is still a beta feature, but that
>> would break existing Mesa releases.
>>
>> Regards,
>> Christian.
>>
>> On 3/24/26 14:40, Liang, Prike wrote:
>>> [Public]
>>>
>>> It's not too much change, so ping?
>>>
>>> Regards,
>>>       Prike
>>>
>>>> -----Original Message-----
>>>> From: Liang, Prike <Prike.Liang@amd.com>
>>>> Sent: Monday, March 23, 2026 11:30 AM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
>>>> <Christian.Koenig@amd.com>; Liang, Prike <Prike.Liang@amd.com>
>>>> Subject: [PATCH] drm/amdgpu: validate SIGNAL/WAIT ioctl input
>>>> argument
>>>>
>>>> Filter out the invalid userq emit and wait ioctl input arguments.
>>>>
>>>> Signed-off-by: Prike Liang <Prike.Liang@amd.com>
>>>> ---
>>>>  .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 27 +++++++++++++++++++
>>>>  1 file changed, 27 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> index f93da45cfa7e..7b2700a0c0ad 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> @@ -483,6 +483,17 @@ int amdgpu_userq_signal_ioctl(struct drm_device
>>>> *dev, void *data,
>>>>       if (args->num_bo_write_handles > AMDGPU_USERQ_MAX_HANDLES ||
>>>>           args->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES)
>>>>               return -EINVAL;
>>>> +     /* After the mesa allocates the input obj properly, then there
>>>> +      * also requires filtering out the invalid obj number.
>>>> +      */
>>>> +     if (args->num_syncobj_handles && !args->syncobj_handles)
>>>> +             return -EINVAL;
>>>> +
>>>> +     if (args->num_bo_read_handles && !args->bo_read_handles)
>>>> +             return -EINVAL;
>>>> +
>>>> +     if (args->num_bo_write_handles && !args->bo_write_handles)
>>>> +             return -EINVAL;
>>>>
>>>>       num_syncobj_handles = args->num_syncobj_handles;
>>>>       syncobj_handles = memdup_array_user(u64_to_user_ptr(args-
>>>>> syncobj_handles),
>>>> @@ -946,6 +957,22 @@ int amdgpu_userq_wait_ioctl(struct drm_device
>>>> *dev, void *data,
>>>>           wait_info->num_bo_read_handles >
>>>> AMDGPU_USERQ_MAX_HANDLES)
>>>>               return -EINVAL;
>>>>
>>>> +     if (wait_info->num_syncobj_handles && !wait_info->syncobj_handles)
>>>> +             return -EINVAL;
>>>> +
>>>> +     if (wait_info->num_syncobj_timeline_handles &&
>>>> +         !(wait_info->syncobj_timeline_handles || wait_info-
>>>>> syncobj_timeline_points))
>>>> +             return -EINVAL;
>>>> +
>>>> +     if (wait_info->num_bo_read_handles && !wait_info->bo_read_handles)
>>>> +             return -EINVAL;
>>>> +
>>>> +     if (wait_info->num_bo_write_handles && !wait_info->bo_write_handles)
>>>> +             return -EINVAL;
>>>> +
>>>> +     if (!wait_info->num_fences && wait_info->out_fences)
>>>> +             return -EINVAL;
>>>> +
>>>>       num_syncobj = wait_info->num_syncobj_handles;
>>>>       ptr = u64_to_user_ptr(wait_info->syncobj_handles);
>>>>       syncobj_handles = memdup_array_user(ptr, num_syncobj,
>>>> sizeof(u32));
>>>> --
>>>> 2.34.1
>>>
>