On 23/02/2026 10:36, Khatri, Sunil wrote:
On 23-02-2026 03:26 pm, Tvrtko Ursulin wrote:i do not want to redo it for sure :) , could you send the patches for review if you havent yet and add me in CC. Also i am here focusing on fixing the the left over place for the same function.On 23/02/2026 08:21, Khatri, Sunil wrote:On 23-02-2026 01:41 pm, Tvrtko Ursulin wrote:On 23/02/2026 04:31, Khatri, Sunil wrote:Applied the changes @Tvrtko Ursulin <[email protected]>Thank you! I am glad they still applied!Also, you were suggesting in your last week's reply to "Please make sure the changes are made for all memdup_user". There are also patches in that series which convert to drm_gem_objects_lookup() which remove some more, but, for those I think I need to check if they need tweaking after dbce431756f8 ("drm/gem: Make drm_gem_objects_lookup() self-cleaning on failure v6"). Maybe it just works though. I will wait until the ones you merge appear in some branch and check.I am in process to merge only two changes patch no 6 and 7 in the series. There were conflicts but i fixed those but still there are some more memdup_user which i will fix with a patch which are left after these 2 patches gets merged.The before mentioned object lookup and syncpoint lookup. For the latter I also had cleanup back in December but did not send it:commit ddb4985391907155a9cf5ef6f5262c1cc8f334c6 Author: Tvrtko Ursulin <[email protected]> Date: Thu Dec 4 13:56:56 2025 +0000 use syncobj lookup helper in amdgpu_userq_signal_ioctl Signed-off-by: Tvrtko Ursulin <[email protected]> commit b21686063a5858833a09bb8f84cfbae2376f0c18 Author: Tvrtko Ursulin <[email protected]> Date: Thu Dec 4 13:36:15 2025 +0000 export sync obj lookup helper Signed-off-by: Tvrtko Ursulin <[email protected]> Those two I was planning to send together with some more syncobj patches: 27cf1f1dabfc drm/syncobj: Add a fast path to drm_syncobj_array_finde7efbf50342b drm/syncobj: Add a fast path to drm_syncobj_array_wait_timeout 363279cd8eb4 drm/syncobj: Use vmemdup_array_user in drm_syncobj_array_findWhich are interesting for graphics since four stack fast path slots avoid 99% of temporary allocations. Not sure for user queues how many sync points are typical.Also if you want to take that up its ok with me. Let me know.¯\_(ツ)_/¯Not sure if you want to re-do it because what I sent has a flaw somewhere or something else?Example below, these changes are still left even after your two changes that i pushed. So i am assuming the patches you mentioned have these changes accomodated this way or some other. Please send them for review.
Got it, thanks! I've sent the rebased series. You can pick and choose from it but please also test the userq part since I only tested the standard submission path.
Regards, Tvrtko
+ 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,14 +666,15 @@ 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));Regards Sunil khatriRegards, TvrtkoOn 20-02-2026 06:49 pm, Khatri, Sunil wrote:On 20-02-2026 04:54 pm, Tvrtko Ursulin wrote: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.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- [email protected]/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 changesAnd later:https://lore.kernel.org/amd-gfx/20260202125149.2067-7- [email protected]/.on the existing code base first. regards Sunil khatriRegards, TvrtkoSigned-off-by: Sunil Khatri <[email protected]> ---.../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.cindex 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;
