On 3/17/26 02:17, Jesse.Zhang wrote:
> Tighten the early parameter checks in the USERQ SIGNAL and WAIT ioctls:
>
> 1. Validate num_syncobj_handles against AMDGPU_USERQ_MAX_HANDLES in
> addition to the BO handle counts that are already checked. The UAPI
> field is __u64 but the driver stores it in a u32, so the comparison
> must happen before the narrowing assignment to prevent unintended
> truncation (e.g. 0x1_0000_0000 would silently become 0).
>
> 2. Reject inconsistent pointer/count pairs where a non-NULL userspace
> pointer is provided with a zero element count. This is clearly
> malformed input and returning -EINVAL early gives userspace a
> deterministic error rather than silently proceeding with empty data.
>
> No functional change for well-formed userspace callers.
>
> v2:
> - Reworked commit message to focus on parameter validation correctness
> - Updated code comments for clarity: describe the type width mismatch
> and why the early check is needed
> - No functional changes to the code itself
>
> Signed-off-by: Jesse Zhang <[email protected]>
> Reviewed-by: Vitaly Prosyak <[email protected]>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 37 ++++++++++++++++++-
> 1 file changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index fad595401a77..575dd58ed152 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -480,8 +480,25 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev,
> void *data,
> if (!amdgpu_userq_enabled(dev))
> return -ENOTSUPP;
>
> + /*
> + * num_syncobj_handles is __u64 in the UAPI but stored in a u32
> + * in the driver. Check all three handle counts against the
> + * maximum *before* the narrowing assignment so that values
> + * above 2^32 are correctly rejected instead of being silently
> + * truncated to a smaller (possibly zero) value.
> + */
That is completely unnecessary, just reduce the variables size in the UAPI as
we already did for num_syncobj_handles.
> if (args->num_bo_write_handles > AMDGPU_USERQ_MAX_HANDLES ||
> - args->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES)
> + args->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES ||
> + args->num_syncobj_handles > AMDGPU_USERQ_MAX_HANDLES) {
You don't need to check num_syncobj_handles here.
Regards,
Christian.
> + return -EINVAL;
> + }
> +
> + /* Reject non-NULL pointers paired with a zero count. */
> + 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;
> @@ -639,7 +656,23 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void
> *data,
> return -ENOTSUPP;
>
> if (wait_info->num_bo_write_handles > AMDGPU_USERQ_MAX_HANDLES ||
> - wait_info->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES)
> + wait_info->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES ||
> + wait_info->num_syncobj_handles > AMDGPU_USERQ_MAX_HANDLES)
> + return -EINVAL;
> +
> + /* Reject non-NULL pointers paired with a zero count: the pointer
> + * is meaningless and indicates inconsistent input from userspace.
> + */
> + 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;
