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

w15303746062@163.com posted 1 patch 2 days, 13 hours ago
drivers/gpu/drm/drm_gem.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
[PATCH v2] drm/gem: fix signed integer overflow in idr_alloc end parameter
Posted by w15303746062@163.com 2 days, 13 hours ago
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