Re: [PATCH] drm/gem: fix warning in idr_alloc due to unvalidated user handle

[Thomas Zimmermann]

Hi

Am 05.06.26 um 05:06 schrieb 王明煜:
> Hi maintainers,
>
> Just a gentle ping on this patch. It fixes a WARN triggered during fuzzing when negative user handles are passed.
>
> Please let me know if it needs any revisions or if there is anything else I can do to help move it forward.
>
> Thanks,
> Mingyu
>
>
> > -----原始邮件-----
> > 发件人: "Mingyu Wang" <25181214217@xxxxxxxxxxxxxxxxx>
> > 发送时间:2026-04-22 19:42:47 (星期三)
> > 收件人: maarten.lankhorst@xxxxxxxxxxxxxxx, mripard@xxxxxxxxxx, tzimmermann@xxxxxxx, airlied@xxxxxxxxx, simona@xxxxxxxx
> > 抄送: dri-devel@xxxxxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, "Mingyu Wang" <25181214217@xxxxxxxxxxxxxxxxx>
> > 主题: [PATCH] drm/gem: fix warning in idr_alloc due to unvalidated user handle
> >
> > During fuzzing, a warning was triggered in idr_alloc() when handling
> > the DRM_IOCTL_GEM_CHANGE_HANDLE (or similar) ioctl.
> >
> > The function drm_gem_change_handle_ioctl() currently only checks if
> > args->new_handle is strictly greater than INT_MAX. However, it fails
> > to check for negative values. If a userpace application passes a
> > negative handle, it bypasses the upper-bound check and is passed
> > directly to idr_alloc() as the 'start' parameter, triggering the
> > WARN_ON_ONCE(start < 0) inside idr_alloc().

args->new_handle is unsigned.  IIRC, for the test, INT_MAX should be 
interpreted as unsigned as well. So how can it get across the INT_MAX 
test? There's an explicit cast to int at [1], which might have an effect 
here.

Does it work of you explicitly cast INT_MAX to u32 in that test?

I'm also worried about interpreting the handle as signed and then adding 
+1 to it. [2]  idr_alloc() appears to handle it gracefully, [3] but it 
still looks fishy.

[1] https://elixir.bootlin.com/linux/v7.0.11/source/include/vdso/limits.h#L8
[2] 
https://elixir.bootlin.com/linux/v7.0.11/source/drivers/gpu/drm/drm_gem.c#L1033
[3] https://elixir.bootlin.com/linux/v7.0.11/source/lib/idr.c#L89

Best regards
Thomas

> > Fix this by explicitly validating that the user-provided handle is
> > strictly positive and within the valid IDR range.
> >
> > Signed-off-by: Mingyu Wang <25181214217@xxxxxxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/drm_gem.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index d6424267260b..3d84d4f1c3e0 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -1026,7 +1026,7 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data,
> >   		return -EOPNOTSUPP;
> >   
> >   	/* idr_alloc() limitation. */
> > -	if (args->new_handle > INT_MAX)
> > +	if (args->new_handle <= 0 || args->new_handle > INT_MAX)
> >   		return -EINVAL;
> >   	handle = args->new_handle;
> >   
> > --
> > 2.34.1

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)