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

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

[w15303746062]

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

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

[w15303746062]

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

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

[w15303746062]

>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.

Hi all,

Following up on my previous email, I just noticed that
drm_gem_change_handle_ioctl has recently been disabled in mainline due to
security and testing concerns, and a TODO list has been added for its
potential re-enablement.

Given this upstream status, I will drop my v2 patch submission. A standalone
fix for the integer overflow no longer applies cleanly without addressing
the full TODO list (including writing the missing IGT tests and handling
idr_preload).

For the mailing list record, and for whoever takes on the re-enablement
task in the future, I want to leave my local findings here regarding the
edge cases:

1. Signed Integer Overflow in idr_alloc:
The original `args->new_handle > INT_MAX` check is insufficient. If
`new_handle` is exactly `INT_MAX`, the subsequent call to `idr_alloc()`
using `handle + 1` as the end limit will result in a signed integer overflow
(-2147483648). The boundary check must be tightened to `>= INT_MAX`.

2. The "0" Handle Semantics (TODO #3):
The original code did not reject handle 0. In the DRM subsystem, 0 is
universally treated as an invalid handle (e.g., `drm_gem_object_lookup`
returns NULL). Allowing an object to be aliased to handle 0 breaks core
DRM state assumptions. An explicit `args->new_handle == 0` check is
mandatory.

Thank you Thomas and the reviewers for your time and the insightful
discussions. I will move on to other security research targets.

Thanks,
Mingyu