Potential NULL pointer dereference in radeon_ttm_tt_populate

[Shaobo He]

 > See here:
 > #if IS_ENABLED(CONFIG_AGP)
 >          if (rdev->flags & RADEON_IS_AGP) {
 >                  return ttm_agp_tt_populate(ttm, ctx);
 >          }
 > #endif
 >

This code appears to be after the potential location of NULL pointer dereference 
that I pointed out. Please see,

```
	if (slave && ttm->sg) {
		drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
					   gtt->ttm.dma_address,ttm->num_pages);
		ttm->state = tt_unbound;
		return 0;
	}
```
The argument expression `gtt->ttm.dma_address` is the potentially offending code 
if `gtt` is NULL.

Shaobo
On 3/19/19 12:28 PM, Christian König wrote:
>> ... or the backend methods is not `radeon_backend_func`.
> That's the case when it is an AGP backend.
> 
>> Moreover, could you point out the check of such case before the offending code?
> 
> See here:
> #if IS_ENABLED(CONFIG_AGP)
>          if (rdev->flags & RADEON_IS_AGP) {
>                  return ttm_agp_tt_populate(ttm, ctx);
>          }
> #endif
> 
> 
>> Meaning the check of whether `ttm` is an AGP ttm?
> Well exactly that's the point, we never check that the ttm structure is an AGP ttm.
> 
> We check if the device is an AGP device and if that's the case then the ttm 
> structure must be an AGP structure as well.
> 
> Regards,
> Christian.
> 
> 
> Am 19.03.19 um 17:46 schrieb Shaobo He:
>> Hi Christian,
>>
>> Thank you very much for your reply. I'm a little confused here so I really 
>> appreciate if you could clarify it more.
>>
>> For example, I don't understand why the condition of function 
>> `radeon_ttm_tt_to_gtt` returning NULL is the argument being an AGP ttm. Based 
>> on its definition, it returns NULL when the argument is NULL or the backend 
>> methods is not `radeon_backend_func`. Is there any correlation that I missed 
>> here?
>>
>> Moreover, could you point out the check of such case before the offending 
>> code? Meaning the check of whether `ttm` is an AGP ttm?
>>
>> Best,
>> Shaobo
>> On 2019/3/19 3:16, Christian König wrote:
>>> Hi Shaobo,
>>>
>>> that question came up a couple of times now. And the answer is: No, there 
>>> can't be a NULL pointer dereference.
>>>
>>> The function radeon_ttm_tt_to_gtt returns NULL only when it is an AGP ttm 
>>> structure, and that case is checked right before the offending code.
>>>
>>> Unfortunately I don't see how an automated code checker should ever be able 
>>> to figure that out by itself.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 18.03.19 um 21:58 schrieb Shaobo He:
>>>> Hello everyone,
>>>>
>>>> My name is Shaobo He and I am a graduate student at University of Utah. I am 
>>>> using a static analysis tool to search for null pointer dereferences and 
>>>> came across a potentially invalid memory access in the file 
>>>> drivers/gpu/drm/radeon/radeon_ttm.c: in function `radeon_ttm_tt_populate`, 
>>>> function `radeon_ttm_tt_to_gtt` can return a NULL pointer which is 
>>>> dereferenced by the call to `drm_prime_sg_to_page_addr_arrays`.
>>>>
>>>> Please let me know if it makes sense. I am looking forward to your reply.
>>>>
>>>> Best,
>>>> Shaobo
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx