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. + */ 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) { + 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; -- 2.49.0
