Re:[PATCH v2] drm/gem: fix signed integer overflow in idr_alloc end parameter

[w15303746062 <w15303746062@163.com>]

Hi maintainers,

Gentle ping on this v2 patch. 

As a quick note regarding the automated review from Sashiko AI: it correctly flagged two pre-existing issues in this function (a missing idr_preload and a critical UAF race condition during DRM_IOCTL_GEM_CLOSE). 

I want to clarify that those are orthogonal to the integer overflow fixed in this v2 patch. I am keeping this patch purely focused on the signed integer overflow issue so it can be evaluated on its own merits. 

I am currently investigating the UAF race condition separately and will submit a new patch series to address the pre-existing bugs flagged by the bot.

Please let me know if any further revisions are needed for this v2 overflow fix.

Thanks,
Mingyu

At 2026-06-05 22:26:44, w15303746062@163.com wrote:
>From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>
>During code review of drm_gem_change_handle_ioctl(), a signed integer
>overflow vulnerability was identified.
>
>The function currently checks if args->new_handle is strictly greater
>than INT_MAX. However, if args->new_handle is exactly INT_MAX, the
>subsequent call to idr_alloc() uses 'handle + 1' as the end limit.
>
>Since 'handle' is a signed int, passing INT_MAX results in a signed
>integer overflow (INT_MAX + 1 = -2147483648). While idr_alloc() handles
>negative end values by gracefully falling back to INT_MAX, this breaks
>the exact-ID allocation semantics intended by the caller and relies on
>undefined behavior.
>
>Additionally, explicitly reject args->new_handle == 0. In the DRM
>subsystem, 0 is universally treated as an invalid handle. Allowing an
>object to be aliased to handle 0 breaks core DRM assumptions (e.g.,
>drm_gem_object_lookup returns NULL for 0).
>
>Fix this by tightening the limit check to >= INT_MAX to prevent the
>overflow, and explicitly reject 0.
>
>Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>---
>v2:
> - Add explicit check to reject handle 0, which is invalid in DRM.
> - Remove redundant (u32) cast; rely on standard C integer promotion.
> - Shift focus entirely to fixing the handle + 1 signed integer overflow (Thomas Zimmermann).
>
> drivers/gpu/drm/drm_gem.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>index e12cdf91f4dc..1ad30a700068 100644
>--- a/drivers/gpu/drm/drm_gem.c
>+++ b/drivers/gpu/drm/drm_gem.c
>@@ -1025,8 +1025,12 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data,
> 	if (!drm_core_check_feature(dev, DRIVER_GEM))
> 		return -EOPNOTSUPP;
> 
>-	/* idr_alloc() limitation. */
>-	if (args->new_handle > INT_MAX)
>+	/*
>+	 * idr_alloc() limitation.
>+	 * Reject handle 0 (invalid in DRM) and strictly bound
>+	 * to < INT_MAX to avoid signed integer overflow in handle + 1.
>+	 */
>+	if (args->new_handle == 0 || args->new_handle >= INT_MAX)
> 		return -EINVAL;
> 	handle = args->new_handle;
> 
>-- 
>2.34.1