[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Koenig, Christian <[email protected]> > Sent: Thursday, March 12, 2026 5:50 PM > To: Zhang, Jesse(Jie) <[email protected]>; [email protected] > Cc: Deucher, Alexander <[email protected]> > Subject: Re: [PATCH] drm/amdgpu: add overflow check for BO list array > allocation > > On 3/12/26 10:48, Zhang, Jesse(Jie) wrote: > > [AMD Official Use Only - AMD Internal Distribution Only] > > > >> -----Original Message----- > >> From: Koenig, Christian <[email protected]> > >> Sent: Thursday, March 12, 2026 5:31 PM > >> To: Zhang, Jesse(Jie) <[email protected]>; > >> [email protected] > >> Cc: Deucher, Alexander <[email protected]> > >> Subject: Re: [PATCH] drm/amdgpu: add overflow check for BO list array > >> allocation > >> > >> On 3/12/26 10:27, Zhang, Jesse(Jie) wrote: > >>> [AMD Official Use Only - AMD Internal Distribution Only] > >>> > >>>> -----Original Message----- > >>>> From: Koenig, Christian <[email protected]> > >>>> Sent: Thursday, March 12, 2026 5:18 PM > >>>> To: Zhang, Jesse(Jie) <[email protected]>; > >>>> [email protected] > >>>> Cc: Deucher, Alexander <[email protected]> > >>>> Subject: Re: [PATCH] drm/amdgpu: add overflow check for BO list > >>>> array allocation > >>>> > >>>> On 3/12/26 09:33, Zhang, Jesse(Jie) wrote: > >>>>> [AMD Official Use Only - AMD Internal Distribution Only] > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Koenig, Christian <[email protected]> > >>>>>> Sent: Thursday, March 12, 2026 4:23 PM > >>>>>> To: Zhang, Jesse(Jie) <[email protected]>; > >>>>>> [email protected] > >>>>>> Cc: Deucher, Alexander <[email protected]> > >>>>>> Subject: Re: [PATCH] drm/amdgpu: add overflow check for BO list > >>>>>> array allocation > >>>>>> > >>>>>> On 3/12/26 09:18, Jesse.Zhang wrote: > >>>>>>> When allocating memory for a BO list array, the multiplication > >>>>>>> bo_number * info_size may overflow on 32-bit systems if > >>>>>>> userspace supplies large values. This could lead to allocating a > >>>>>>> smaller buffer than expected, followed by a memset or > >>>>>>> copy_from_user that writes beyond the allocated memory, > >>>>>>> potentially causing memory corruption or information disclosure. > >>>>>>> > >>>>>>> Add an overflow check using check_mul_overflow to detect such cases. > >>>>>>> Also ensure the resulting allocation size does not exceed > >>>>>>> INT_MAX, as the subsequent user copy operations may rely on this > >>>>>>> limit. > >>>>>>> Return -EINVAL if either condition fails. > >>>>>> > >>>>>> That is completely unnecessary, vmemdup_array_user() already does > >>>>>> that > >>>> check. > >>>>>> > >>>>>>> > >>>>>>> A crash log illustrating the issue: > >>>>>>> > >>>>>>> [ 2943.053706] RIP: 0010:__kvmalloc_node_noprof+0x5be/0x8a0 > >>>>>>> ... > >>>>>>> [ 2943.053725] Call Trace: > >>>>>>> [ 2943.053728] amdgpu_bo_create_list_entry_array+0x42/0x130 > >>>>>>> [amdgpu] [ 2943.053947] amdgpu_bo_list_ioctl+0x51/0x300 [amdgpu] > >>>>>>> [ 2943.054277] > >>>>>>> drm_ioctl+0x2cb/0x5a0 [drm] [ 2943.054379] > >>>>>>> __x64_sys_ioctl+0x9e/0xf0 > >>>>>>> > >>>>>>> The overflow occurs in the allocation inside > >>>>>>> amdgpu_bo_create_list_entry_array, leading to a crash in > >>>>>>> vmemdup_user (via __kvmalloc_node_noprof). > >>>>>> > >>>>>> How and on which kernel can you reproduce that? > >>>>> We are developing some fuzz tests for the unified project. > >>>>> The tests involve passing different levels of garbage data and > >>>>> ensuring the kernel > >>>> can handle this data correctly. > >>>>> This issue can be reproduced on the amd-staging-drm-next branch. > >>>> > >>>> Do you have the full backtrace? > >>> Yes, > >>> [ 2943.053649] WARNING: mm/slub.c:7152 at > >>> __kvmalloc_node_noprof+0x5be/0x8a0, CPU#13: amd_fuzzing/2765 > >> > >> Ah, yes. That problem came up before. > >> > >> The maximum number of BOs in a BO list should be limited and not the > >> result of the multiplication checked. > >> > >> The problem is that we couldn't give a good number on the maximum BOs > >> we can have in a BO list. > > Thanks Chritian, agreed. v2 switches from pure multiplication-overflow > > wording to a BO-count limit. We now bound bo_number by INT_MAX / > > sizeof(drm_amdgpu_bo_list_entry) before allocation/copy. This keeps > > behavior deterministic for fuzzed input and avoids warning-prone huge > > allocation paths > > Yeah, but I've rejected that before as well. INT_MAX / > sizeof(drm_amdgpu_bo_list_entry) is just not a good maximum limit. > > This needs to be a fixed constant. Thanks Christian, agreed — we should use a fixed limit, not an INT_MAX-derived one. I switched the patch to a constant BO-list cap and kept the bo_info_size sanity check. bo_number is now rejected with -EINVAL when it exceeds AMDGPU_BO_LIST_MAX_ENTRIES (currently set to 128 * 1024).
Thanks Jesse > > Regards, > Christian. > > > > > Thanks > > Jesse > > > >> > >> Regards, > >> Christian.
