[PATCH 1/2] drm/amdgpu: fix NULL pointer dereference

[Christian König]

Am 30.03.21 um 15:15 schrieb Chen, Guchun:
> [AMD Public Use]
>
> Thanks Christian, I will put laser focus on this patch after merging it.
>
> I notice the same logic in radeon code radeon_ttm_tt_unpin_userptr. Shall I create another patch to fix it as well?

If you have time, then please do so. Cause those bugs are on my todo 
list for quite a while and I couldn't find time to fix them.

Regards,
Christian.

>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken at gmail.com>
> Sent: Tuesday, March 30, 2021 6:39 PM
> To: Chen, Guchun <Guchun.Chen at amd.com>; amd-gfx at lists.freedesktop.org; Koenig, Christian <Christian.Koenig at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>
> Subject: Re: [PATCH 1/2] drm/amdgpu: fix NULL pointer dereference
>
> Am 30.03.21 um 12:02 schrieb Guchun Chen:
>> ttm->sg needs to be checked before accessing its child member.
>>
>> Call Trace:
>>    amdgpu_ttm_backend_destroy+0x12/0x70 [amdgpu]
>>    ttm_bo_cleanup_memtype_use+0x3a/0x60 [ttm]
>>    ttm_bo_release+0x17d/0x300 [ttm]
>>    amdgpu_bo_unref+0x1a/0x30 [amdgpu]
>>    amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu+0x78b/0x8b0 [amdgpu]
>>    kfd_ioctl_alloc_memory_of_gpu+0x118/0x220 [amdgpu]
>>    kfd_ioctl+0x222/0x400 [amdgpu]
>>    ? kfd_dev_is_large_bar+0x90/0x90 [amdgpu]
>>    __x64_sys_ioctl+0x8e/0xd0
>>    ? __context_tracking_exit+0x52/0x90
>>    do_syscall_64+0x33/0x80
>>    entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> RIP: 0033:0x7f97f264d317
>> Code: b3 66 90 48 8b 05 71 4b 2d 00 64 c7 00 26 00 00 00 48 c7 c0 ff
>> ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d
>> 01 f0 ff ff 73 01 c3 48 8b 0d 41 4b 2d 00 f7 d8 64 89 01 48
>> RSP: 002b:00007ffdb402c338 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
>> RAX: ffffffffffffffda RBX: 00007f97f3cc63a0 RCX: 00007f97f264d317
>> RDX: 00007ffdb402c380 RSI: 00000000c0284b16 RDI: 0000000000000003
>> RBP: 00007ffdb402c380 R08: 00007ffdb402c428 R09: 00000000c4000004
>> R10: 00000000c4000004 R11: 0000000000000246 R12: 00000000c0284b16
>> R13: 0000000000000003 R14: 00007f97f3cc63a0 R15: 00007f8836200000
>>
>> Signed-off-by: Guchun Chen <guchun.chen at amd.com>
> Yeah I had this one on my TODO list as well.
>
> For now the patch is Acked-by: Christian König <christian.koenig at amd.com>, but I'm not 100% sure if this is the right fix.
>
> Please keep an eye open if anybody complains about issues with this patch, if yes we need to get back to the drawing board.
>
> Christian.
>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index e00263bcc88b..722efd86718e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -867,7 +867,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_device *bdev,
>>    		DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
>>    
>>    	/* double check that we don't free the table twice */
>> -	if (!ttm->sg->sgl)
>> +	if (!ttm->sg || !ttm->sg->sgl)
>>    		return;
>>    
>>    	/* unmap the pages mapped to the device */