Re: [PATCH] drm/amdgpu: Fix out-of-bounds write warning

["Christian König" <christian.koenig@amd.com>]

Am 26.04.24 um 05:24 schrieb Ma, Jun:
> On 4/25/2024 8:39 PM, Christian König wrote:
>> Am 25.04.24 um 12:00 schrieb Ma Jun:
>>> Check the ring type value to fix the out-of-bounds
>>> write warning
>>>
>>> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>> index 06f0a6534a94..1e0b5bb47bc9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>> @@ -353,6 +353,11 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
>>>    	ring->hw_prio = hw_prio;
>>>    
>>>    	if (!ring->no_scheduler) {
>>> +		if (ring->funcs->type >= AMDGPU_HW_IP_NUM) {
>>> +			dev_warn(adev->dev, "ring type %d has no scheduler\n", ring->funcs->type);
>>> +			return 0;
>>> +		}
>>> +
>> That check should probably be at the beginning of the function since
>> trying to initialize a ring with an invalid type should be rejected
>> immediately.
>>
> This check is used to skip the gpu_sched setting for the rings
> which don't have scheduler, such as KIQ, MES, UMSCH_MM.
> Without this check, there could be an potential out-of-bounds writing
> when ring->no__scheduler is not set correctly.

Ah! In this case that that is not really clean.

Probably best approach is to change the "if (!ring->no_scheduler)" instead.

Maybe even move this code here into amdgpu_ctx.c:

                 hw_ip = ring->funcs->type;
                 num_sched = &adev->gpu_sched[hw_ip][hw_prio].num_scheds;
adev->gpu_sched[hw_ip][hw_prio].sched[(*num_sched)++] =
                         &ring->sched;

And then add something like hw_ip <= AMDGPU_HW_IP_NUM.

Background is that it is perfectly valid to have a scheduler for the MES 
for example.

Regards,
Christian.

>
> Regards,
> Ma Jun
>
>> Regards,
>> Christian.
>>
>>>    		hw_ip = ring->funcs->type;
>>>    		num_sched = &adev->gpu_sched[hw_ip][hw_prio].num_scheds;
>>>    		adev->gpu_sched[hw_ip][hw_prio].sched[(*num_sched)++] =