[PATCH v5 0/4] signal and wait ioctl fixes

[PATCH v5 0/4] signal and wait ioctl fixes

[Sunil Khatri]

v5:
Only patch 4/4 is updated:
Removed the syncobj_timeline_points as its not needed and also its not
not count but a VA.


Sunil Khatri (4):
  drm/amdgpu/userq: initialize gobj_read/write in
    amdgpu_userq_signal_ioctl
  drm/amdgpu/userq: initialize gobj_read/write in
    amdgpu_userq_wait_ioctl
  drm/amdgpu: add upper bound check on user inputs in signal ioctl
  drm/amdgpu: add upper bound check on user inputs in wait ioctl

 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v5 1/4] drm/amdgpu/userq: initialize gobj_read/write in amdgpu_userq_signal_ioctl

[Sunil Khatri]

In case num_read_bo_handles or num_write_bo_handles is zero the ptrs
remain uninitialized and during free cause a fault. So to handle such
cases we better set the gobj_read and gobj_write to NULL.

Fixes: 3cf117572294 ("drm/amdgpu/userq: Use drm_gem_objects_lookup in amdgpu_userq_signal_ioctl")
Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 18e77b61b201..e53e14e3bf2d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -465,7 +465,7 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
 	const unsigned int num_read_bo_handles = args->num_bo_read_handles;
 	struct amdgpu_fpriv *fpriv = filp->driver_priv;
 	struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
-	struct drm_gem_object **gobj_write, **gobj_read;
+	struct drm_gem_object **gobj_write = NULL, **gobj_read = NULL;
 	u32 *syncobj_handles, num_syncobj_handles;
 	struct amdgpu_userq_fence *userq_fence;
 	struct amdgpu_usermode_queue *queue;
-- 
2.34.1

[PATCH v5 2/4] drm/amdgpu/userq: initialize gobj_read/write in amdgpu_userq_wait_ioctl

[Sunil Khatri]

In case num_read_bo_handles or num_write_bo_handles is zero the ptrs
remain uninitialized and during free cause a fault. So to handle such
cases we better set the gobj_read and gobj_write to NULL.

Fixes: 42e01090a47c ("drm/amdgpu/userq: Use drm_gem_objects_lookup in amdgpu_userq_wait_ioctl")
Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index e53e14e3bf2d..42b548c8a86e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -617,7 +617,7 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
 	struct drm_amdgpu_userq_fence_info *fence_info = NULL;
 	struct amdgpu_fpriv *fpriv = filp->driver_priv;
 	struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
-	struct drm_gem_object **gobj_write, **gobj_read;
+	struct drm_gem_object **gobj_write = NULL, **gobj_read = NULL;
 	u32 *timeline_points, *timeline_handles;
 	struct amdgpu_usermode_queue *waitq;
 	u32 *syncobj_handles, num_syncobj;
-- 
2.34.1

[PATCH v5 3/4] drm/amdgpu: add upper bound check on user inputs in signal ioctl

[Sunil Khatri]

Huge input values in amdgpu_userq_signal_ioctl can lead to a OOM and
could be exploited.

So check these input value against AMDGPU_USERQ_MAX_HANDLES
which is big enough value for genuine use cases and could
potentially avoid OOM.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 42b548c8a86e..4f9386677c47 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -35,6 +35,8 @@
 static const struct dma_fence_ops amdgpu_userq_fence_ops;
 static struct kmem_cache *amdgpu_userq_fence_slab;
 
+#define AMDGPU_USERQ_MAX_HANDLES	(1U << 16)
+
 int amdgpu_userq_fence_slab_init(void)
 {
 	amdgpu_userq_fence_slab = kmem_cache_create("amdgpu_userq_fence",
@@ -478,6 +480,11 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
 	if (!amdgpu_userq_enabled(dev))
 		return -ENOTSUPP;
 
+	if (args->num_syncobj_handles > AMDGPU_USERQ_MAX_HANDLES ||
+	    args->num_bo_write_handles > AMDGPU_USERQ_MAX_HANDLES ||
+	    args->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES)
+		return -EINVAL;
+
 	num_syncobj_handles = args->num_syncobj_handles;
 	syncobj_handles = memdup_array_user(u64_to_user_ptr(args->syncobj_handles),
 					    num_syncobj_handles, sizeof(u32));
-- 
2.34.1

[PATCH v5 4/4] drm/amdgpu: add upper bound check on user inputs in wait ioctl

[Sunil Khatri]

Huge input values in amdgpu_userq_wait_ioctl can lead to a OOM and
could be exploited.

So check these input value against AMDGPU_USERQ_MAX_HANDLES
which is big enough value for genuine use cases and could
potentially avoid OOM.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 4f9386677c47..e31b2c6cc73b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -636,6 +636,12 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
 	if (!amdgpu_userq_enabled(dev))
 		return -ENOTSUPP;
 
+	if (wait_info->num_syncobj_handles > AMDGPU_USERQ_MAX_HANDLES ||
+	    wait_info->num_syncobj_timeline_handles > AMDGPU_USERQ_MAX_HANDLES ||
+	    wait_info->num_bo_write_handles > AMDGPU_USERQ_MAX_HANDLES ||
+	    wait_info->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES)
+		return -EINVAL;
+
 	num_syncobj = wait_info->num_syncobj_handles;
 	syncobj_handles = memdup_array_user(u64_to_user_ptr(wait_info->syncobj_handles),
 					    num_syncobj, sizeof(u32));
-- 
2.34.1

Re: [PATCH v5 1/4] drm/amdgpu/userq: initialize gobj_read/write in amdgpu_userq_signal_ioctl

[Christian König]

On 2/24/26 10:11, Sunil Khatri wrote:
> In case num_read_bo_handles or num_write_bo_handles is zero the ptrs
> remain uninitialized and during free cause a fault. So to handle such
> cases we better set the gobj_read and gobj_write to NULL.

Yeah that still doesn't looks like a good idea to me.

We intentionally avoid nationalizations like that if they aren't necessary because that allows the compiler to complain about it.

Christian.

> 
> Fixes: 3cf117572294 ("drm/amdgpu/userq: Use drm_gem_objects_lookup in amdgpu_userq_signal_ioctl")
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 18e77b61b201..e53e14e3bf2d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -465,7 +465,7 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>  	const unsigned int num_read_bo_handles = args->num_bo_read_handles;
>  	struct amdgpu_fpriv *fpriv = filp->driver_priv;
>  	struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
> -	struct drm_gem_object **gobj_write, **gobj_read;
> +	struct drm_gem_object **gobj_write = NULL, **gobj_read = NULL;
>  	u32 *syncobj_handles, num_syncobj_handles;
>  	struct amdgpu_userq_fence *userq_fence;
>  	struct amdgpu_usermode_queue *queue;

Re: [PATCH v5 1/4] drm/amdgpu/userq: initialize gobj_read/write in amdgpu_userq_signal_ioctl

[Khatri, Sunil]

On 24-02-2026 02:58 pm, Christian König wrote:
> On 2/24/26 10:11, Sunil Khatri wrote:
>> In case num_read_bo_handles or num_write_bo_handles is zero the ptrs
>> remain uninitialized and during free cause a fault. So to handle such
>> cases we better set the gobj_read and gobj_write to NULL.
> Yeah that still doesn't looks like a good idea to me.
>
> We intentionally avoid nationalizations like that if they aren't necessary because that allows the compiler to complain about it.
>
> Christian.
Sure Christian.
@Alex, can you pull in the drm-misc-next to have the fixes in ASDN, or 
if it is supposed to take some time then in that case i think we need to 
push the change no 1 and 2 to ASDN as signal/wait IOCTL are broken right 
now.

Regards
Sunil Khatri
>
>> Fixes: 3cf117572294 ("drm/amdgpu/userq: Use drm_gem_objects_lookup in amdgpu_userq_signal_ioctl")
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> index 18e77b61b201..e53e14e3bf2d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> @@ -465,7 +465,7 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>>   	const unsigned int num_read_bo_handles = args->num_bo_read_handles;
>>   	struct amdgpu_fpriv *fpriv = filp->driver_priv;
>>   	struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
>> -	struct drm_gem_object **gobj_write, **gobj_read;
>> +	struct drm_gem_object **gobj_write = NULL, **gobj_read = NULL;
>>   	u32 *syncobj_handles, num_syncobj_handles;
>>   	struct amdgpu_userq_fence *userq_fence;
>>   	struct amdgpu_usermode_queue *queue;

Re: [PATCH v5 1/4] drm/amdgpu/userq: initialize gobj_read/write in amdgpu_userq_signal_ioctl

[Christian König]

On 2/24/26 13:50, Khatri, Sunil wrote:
> 
> On 24-02-2026 02:58 pm, Christian König wrote:
>> On 2/24/26 10:11, Sunil Khatri wrote:
>>> In case num_read_bo_handles or num_write_bo_handles is zero the ptrs
>>> remain uninitialized and during free cause a fault. So to handle such
>>> cases we better set the gobj_read and gobj_write to NULL.
>> Yeah that still doesn't looks like a good idea to me.
>>
>> We intentionally avoid nationalizations like that if they aren't necessary because that allows the compiler to complain about it.
>>
>> Christian.
> Sure Christian.
> @Alex, can you pull in the drm-misc-next to have the fixes in ASDN, or if it is supposed to take some time then in that case i think we need to push the change no 1 and 2 to ASDN as signal/wait IOCTL are broken right now.

We just need the ack from Alex to cherry pick the patch from Srini over to ASDN. Should be able to do this by tomorrow.

Regards,
Christian.

> 
> Regards
> Sunil Khatri
>>
>>> Fixes: 3cf117572294 ("drm/amdgpu/userq: Use drm_gem_objects_lookup in amdgpu_userq_signal_ioctl")
>>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> index 18e77b61b201..e53e14e3bf2d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> @@ -465,7 +465,7 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>>>       const unsigned int num_read_bo_handles = args->num_bo_read_handles;
>>>       struct amdgpu_fpriv *fpriv = filp->driver_priv;
>>>       struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
>>> -    struct drm_gem_object **gobj_write, **gobj_read;
>>> +    struct drm_gem_object **gobj_write = NULL, **gobj_read = NULL;
>>>       u32 *syncobj_handles, num_syncobj_handles;
>>>       struct amdgpu_userq_fence *userq_fence;
>>>       struct amdgpu_usermode_queue *queue;

Re: [PATCH v5 1/4] drm/amdgpu/userq: initialize gobj_read/write in amdgpu_userq_signal_ioctl

[Alex Deucher]

On Tue, Feb 24, 2026 at 8:34 AM Christian König
<christian.koenig@amd.com> wrote:
>
> On 2/24/26 13:50, Khatri, Sunil wrote:
> >
> > On 24-02-2026 02:58 pm, Christian König wrote:
> >> On 2/24/26 10:11, Sunil Khatri wrote:
> >>> In case num_read_bo_handles or num_write_bo_handles is zero the ptrs
> >>> remain uninitialized and during free cause a fault. So to handle such
> >>> cases we better set the gobj_read and gobj_write to NULL.
> >> Yeah that still doesn't looks like a good idea to me.
> >>
> >> We intentionally avoid nationalizations like that if they aren't necessary because that allows the compiler to complain about it.
> >>
> >> Christian.
> > Sure Christian.
> > @Alex, can you pull in the drm-misc-next to have the fixes in ASDN, or if it is supposed to take some time then in that case i think we need to push the change no 1 and 2 to ASDN as signal/wait IOCTL are broken right now.
>
> We just need the ack from Alex to cherry pick the patch from Srini over to ASDN. Should be able to do this by tomorrow.

Sure go ahead and cherry-pick what you need.  That said, I think this
patch is fine on its own as well:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

Alex

>
> Regards,
> Christian.
>
> >
> > Regards
> > Sunil Khatri
> >>
> >>> Fixes: 3cf117572294 ("drm/amdgpu/userq: Use drm_gem_objects_lookup in amdgpu_userq_signal_ioctl")
> >>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> >>> ---
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> >>> index 18e77b61b201..e53e14e3bf2d 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> >>> @@ -465,7 +465,7 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> >>>       const unsigned int num_read_bo_handles = args->num_bo_read_handles;
> >>>       struct amdgpu_fpriv *fpriv = filp->driver_priv;
> >>>       struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
> >>> -    struct drm_gem_object **gobj_write, **gobj_read;
> >>> +    struct drm_gem_object **gobj_write = NULL, **gobj_read = NULL;
> >>>       u32 *syncobj_handles, num_syncobj_handles;
> >>>       struct amdgpu_userq_fence *userq_fence;
> >>>       struct amdgpu_usermode_queue *queue;
>

Re: [PATCH v5 4/4] drm/amdgpu: add upper bound check on user inputs in wait ioctl

[Alex Deucher]

On Tue, Feb 24, 2026 at 4:11 AM Sunil Khatri <sunil.khatri@amd.com> wrote:
>
> Huge input values in amdgpu_userq_wait_ioctl can lead to a OOM and
> could be exploited.
>
> So check these input value against AMDGPU_USERQ_MAX_HANDLES
> which is big enough value for genuine use cases and could
> potentially avoid OOM.
>
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 4f9386677c47..e31b2c6cc73b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -636,6 +636,12 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
>         if (!amdgpu_userq_enabled(dev))
>                 return -ENOTSUPP;
>
> +       if (wait_info->num_syncobj_handles > AMDGPU_USERQ_MAX_HANDLES ||
> +           wait_info->num_syncobj_timeline_handles > AMDGPU_USERQ_MAX_HANDLES ||
> +           wait_info->num_bo_write_handles > AMDGPU_USERQ_MAX_HANDLES ||
> +           wait_info->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES)
> +               return -EINVAL;
> +
>         num_syncobj = wait_info->num_syncobj_handles;
>         syncobj_handles = memdup_array_user(u64_to_user_ptr(wait_info->syncobj_handles),
>                                             num_syncobj, sizeof(u32));
> --
> 2.34.1
>