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

[Khatri, Sunil]

On 20-02-2026 06:51 pm, Christian König wrote:
On 2/20/26 14:19, Khatri, Sunil wrote:
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.
I simply couldn't find time to review those patches and then they felt under 
the table.

If Tvrtko came up with the same patch earlier than us then just take that one, 
review it and push to amd-staging-drm-next.

I have reviewed the patches for Tvrtko, change, you can review patch 2 
as that is not related to this.

regards
Sunil Khatri


Thanks,
Christian.

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;