Hi Thomas,

Thank you for the insightful review and suggestions. You were absolutely right 
on both points.

I went back to analyze the Syzkaller reproducer trace. The fuzzer actually 
managed to pass 0xfffffffd as args->new_handle. Due to the explicit (int) cast 
in the INT_MAX macro definition, an implicit type conversion quirk allowed this 
value to bypass the args->new_handle > INT_MAX check, ultimately reaching 
idr_alloc as -3 and triggering the warning.

Testing your suggestion to explicitly cast INT_MAX to u32 successfully catches 
this and prevents the warning.

Regarding your second concern about handle + 1, I completely agree. If handle 
equals INT_MAX, handle + 1 will overflow to a negative value. Although 
idr_alloc handles negative end values by falling back to INT_MAX, it breaks the 
exact-ID allocation logic intended here.

I will send a v2 patch shortly that explicitly casts INT_MAX and tightens the 
boundary check to >= to cover the handle + 1 overflow.

Thanks,
Mingyu


> -----原始邮件-----
> 发件人: "Thomas Zimmermann" <[email protected]>
> 发送时间:2026-06-05 16:02:33 (星期五)
> 收件人: 王明煜 <[email protected]>, [email protected], 
> [email protected], [email protected], [email protected]
> 抄送: [email protected], [email protected]
> 主题: Re: [PATCH] drm/gem: fix warning in idr_alloc due to unvalidated user 
> handle
> 
> 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" <[email protected]>
> >> 发送时间:2026-04-22 19:42:47 (星期三)
> >> 收件人: [email protected], [email protected], 
> >> [email protected], [email protected], [email protected]
> >> 抄送: [email protected], [email protected], "Mingyu 
> >> Wang" <[email protected]>
> >> 主题: [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 <[email protected]>
> >> ---
> >>   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)
> 

Reply via email to