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 <[email protected]> >> Sent: Tuesday, March 24, 2026 10:10 PM >> To: Liang, Prike <[email protected]>; [email protected] >> Cc: Deucher, Alexander <[email protected]> >> 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 <[email protected]> >>>> Sent: Monday, March 23, 2026 11:30 AM >>>> To: [email protected] >>>> Cc: Deucher, Alexander <[email protected]>; Koenig, Christian >>>> <[email protected]>; Liang, Prike <[email protected]> >>>> 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 <[email protected]> >>>> --- >>>> .../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 >>> >
