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, [email protected] wrote: >From: Mingyu Wang <[email protected]> > >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 <[email protected]> >--- >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
