drm/gem: fix warning in idr_alloc due to unvalidated user handle

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

Posted by Mingyu Wang 1 month, 3 weeks ago

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

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@stu.xidian.edu.cn>
---
 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

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

Posted by 王明煜 1 week, 5 days ago

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@stu.xidian.edu.cn>
> 发送时间:2026-04-22 19:42:47 (星期三)
> 收件人: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch
> 抄送: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>
> 主题: [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().
> 
> 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@stu.xidian.edu.cn>
> ---
>  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

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

Posted by Thomas Zimmermann 1 week, 4 days ago

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@stu.xidian.edu.cn>
>> 发送时间:2026-04-22 19:42:47 (星期三)
>> 收件人: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch
>> 抄送: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>
>> 主题: [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@stu.xidian.edu.cn>
>> ---
>>   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)

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

Posted by 王明煜 1 week, 4 days ago

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" <tzimmermann@suse.de>
> 发送时间:2026-06-05 16:02:33 (星期五)
> 收件人: 王明煜 <25181214217@stu.xidian.edu.cn>, maarten.lankhorst@linux.intel.com, mripard@kernel.org, airlied@gmail.com, simona@ffwll.ch
> 抄送: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
> 主题: 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" <25181214217@stu.xidian.edu.cn>
> >> 发送时间:2026-04-22 19:42:47 (星期三)
> >> 收件人: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch
> >> 抄送: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>
> >> 主题: [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@stu.xidian.edu.cn>
> >> ---
> >>   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)
>

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

Posted by 王明煜 1 week, 4 days ago

Hi Thomas,

Apologies for the noise, please disregard my previous email. I spoke too soon.

After re-evaluating the C standard integer promotion rules, you are absolutely right. Since `args->new_handle` is unsigned, `INT_MAX` is promoted to unsigned during the comparison. A negative payload from the fuzzer (e.g., 0xfffffffd) actually evaluates as a massive positive number and is correctly caught by the original `> INT_MAX` check. The implicit conversion bypass I initially suspected does not happen here.

However, your concern about the `handle + 1` overflow is the real issue. If the user passes exactly `INT_MAX`, it bypasses the `>` check, and `INT_MAX + 1` overflows to a negative value when evaluated for `idr_alloc()`. While `idr_alloc()` handles negative end limits by falling back to `INT_MAX`, this breaks the exact-ID allocation semantics intended by the caller.

I have adjusted the patch to focus entirely on preventing this signed integer overflow by tightening the check to `>= INT_MAX`. I will send the v2 patch shortly.

Thanks,
Mingyu


> -----原始邮件-----
> 发件人: 王明煜 <25181214217@stu.xidian.edu.cn>
> 发送时间:2026-06-05 21:38:27 (星期五)
> 收件人: "Thomas Zimmermann" <tzimmermann@suse.de>
> 抄送: maarten.lankhorst@linux.intel.com, mripard@kernel.org, airlied@gmail.com, simona@ffwll.ch, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
> 主题: Re: Re: [PATCH] drm/gem: fix warning in idr_alloc due to unvalidated user handle
> 
> 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" <tzimmermann@suse.de>
> > 发送时间:2026-06-05 16:02:33 (星期五)
> > 收件人: 王明煜 <25181214217@stu.xidian.edu.cn>, maarten.lankhorst@linux.intel.com, mripard@kernel.org, airlied@gmail.com, simona@ffwll.ch
> > 抄送: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
> > 主题: 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" <25181214217@stu.xidian.edu.cn>
> > >> 发送时间:2026-04-22 19:42:47 (星期三)
> > >> 收件人: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch
> > >> 抄送: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>
> > >> 主题: [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@stu.xidian.edu.cn>
> > >> ---
> > >>   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)
> >