On 3/17/26 08:47, 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 > (Vitaly) > - Updated code comments for clarity: describe the type width mismatch > and why the early check is needed (Vitaly) > - No functional changes to the code itself (Vitaly) > > v3: drop the num_syncobj_handles > AMDGPU_USERQ_MAX_HANDLES checks (Christian) > > Signed-off-by: Jesse Zhang <[email protected]> > Reviewed-by: Vitaly Prosyak <[email protected]>
Reviewed-by: Christian König <[email protected]> We should also take another look at the UAPI and make sure that all those num_* fields are either u16 or u32 and not u64. Regards, Christian. > --- > .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 29 +++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > index fad595401a77..5ff8fc815e45 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > @@ -484,6 +484,16 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, > void *data, > args->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES) > 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; > syncobj_handles = > memdup_array_user(u64_to_user_ptr(args->syncobj_handles), > num_syncobj_handles, sizeof(u32)); > @@ -642,6 +652,25 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void > *data, > wait_info->num_bo_read_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; > syncobj_handles = > memdup_array_user(u64_to_user_ptr(wait_info->syncobj_handles), > num_syncobj, sizeof(u32));
