Re: [PATCH v1 1/2] drm/amdgpu: update memdup_user with memdup_array_user

[Khatri, Sunil]

On 20-02-2026 04:54 pm, Tvrtko Ursulin wrote:

On 20/02/2026 08:28, Sunil Khatri wrote:
memdup_user could return invalid memory allocation if
there is an integer overflow. Using memdup_array_user
make sure we validate the size requirements upfront
and return with an error.

FYI:

https://lore.kernel.org/amd-gfx/20251205134035.91551-1-tvrtko.ursu...@igalia.com/ 

Are you awaiting for anything to merge the changes Tvrtko or they are 
merged in drm upstream already? Since issue is reported now so we need 
to fix this.


And later:

https://lore.kernel.org/amd-gfx/20260202125149.2067-7-christian.koe...@amd.com/. 

Yeah i am aware of these Christian changes and we decided we will fix 
the issues first and reorganize the code later as needed along with 
Christian changes. So we are going to push the changes

on the existing code base first.

regards
Sunil khatri


Regards,

Tvrtko


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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 212056d4ddf0..a6eb703b62c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -480,8 +480,8 @@ int amdgpu_userq_signal_ioctl(struct drm_device 
*dev, void *data,
          return -ENOTSUPP;
        num_syncobj_handles = args->num_syncobj_handles;
-    syncobj_handles = 
memdup_user(u64_to_user_ptr(args->syncobj_handles),
-                      size_mul(sizeof(u32), num_syncobj_handles));
+    syncobj_handles = 
memdup_array_user(u64_to_user_ptr(args->syncobj_handles),
+                        num_syncobj_handles, sizeof(u32));
      if (IS_ERR(syncobj_handles))
          return PTR_ERR(syncobj_handles);
  @@ -501,8 +501,8 @@ int amdgpu_userq_signal_ioctl(struct drm_device 
*dev, void *data,
      }
        num_read_bo_handles = args->num_bo_read_handles;
-    bo_handles_read = 
memdup_user(u64_to_user_ptr(args->bo_read_handles),
-                      sizeof(u32) * num_read_bo_handles);
+    bo_handles_read = 
memdup_array_user(u64_to_user_ptr(args->bo_read_handles),
+                        num_read_bo_handles, sizeof(u32));
      if (IS_ERR(bo_handles_read)) {
          r = PTR_ERR(bo_handles_read);
          goto free_syncobj;
@@ -524,8 +524,8 @@ int amdgpu_userq_signal_ioctl(struct drm_device 
*dev, void *data,
      }
        num_write_bo_handles = args->num_bo_write_handles;
-    bo_handles_write = 
memdup_user(u64_to_user_ptr(args->bo_write_handles),
-                       sizeof(u32) * num_write_bo_handles);
+    bo_handles_write = 
memdup_array_user(u64_to_user_ptr(args->bo_write_handles),
+                         num_write_bo_handles, sizeof(u32));
      if (IS_ERR(bo_handles_write)) {
          r = PTR_ERR(bo_handles_write);
          goto put_gobj_read;
@@ -666,37 +666,37 @@ int amdgpu_userq_wait_ioctl(struct drm_device 
*dev, void *data,
          return -ENOTSUPP;
        num_read_bo_handles = wait_info->num_bo_read_handles;
-    bo_handles_read = 
memdup_user(u64_to_user_ptr(wait_info->bo_read_handles),
-                      size_mul(sizeof(u32), num_read_bo_handles));
+    bo_handles_read = 
memdup_array_user(u64_to_user_ptr(wait_info->bo_read_handles),
+                        num_read_bo_handles, sizeof(u32));
      if (IS_ERR(bo_handles_read))
          return PTR_ERR(bo_handles_read);
        num_write_bo_handles = wait_info->num_bo_write_handles;
-    bo_handles_write = 
memdup_user(u64_to_user_ptr(wait_info->bo_write_handles),
-                       size_mul(sizeof(u32), num_write_bo_handles));
+    bo_handles_write = 
memdup_array_user(u64_to_user_ptr(wait_info->bo_write_handles),
+                         num_write_bo_handles, sizeof(u32));
      if (IS_ERR(bo_handles_write)) {
          r = PTR_ERR(bo_handles_write);
          goto free_bo_handles_read;
      }
        num_syncobj = wait_info->num_syncobj_handles;
-    syncobj_handles = 
memdup_user(u64_to_user_ptr(wait_info->syncobj_handles),
-                      size_mul(sizeof(u32), num_syncobj));
+    syncobj_handles = 
memdup_array_user(u64_to_user_ptr(wait_info->syncobj_handles),
+                        num_syncobj, sizeof(u32));
      if (IS_ERR(syncobj_handles)) {
          r = PTR_ERR(syncobj_handles);
          goto free_bo_handles_write;
      }
        num_points = wait_info->num_syncobj_timeline_handles;
-    timeline_handles = 
memdup_user(u64_to_user_ptr(wait_info->syncobj_timeline_handles),
-                       sizeof(u32) * num_points);
+    timeline_handles = 
memdup_array_user(u64_to_user_ptr(wait_info->syncobj_timeline_handles),
+                         num_points, sizeof(u32));
      if (IS_ERR(timeline_handles)) {
          r = PTR_ERR(timeline_handles);
          goto free_syncobj_handles;
      }
  -    timeline_points = 
memdup_user(u64_to_user_ptr(wait_info->syncobj_timeline_points),
-                      sizeof(u32) * num_points);
+    timeline_points = 
memdup_array_user(u64_to_user_ptr(wait_info->syncobj_timeline_points),
+                        num_points, sizeof(u32));
      if (IS_ERR(timeline_points)) {
          r = PTR_ERR(timeline_points);
          goto free_timeline_handles;