Am 19.03.19 um 21:01 schrieb 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.
Yeah, but same thing for this as well.
That code path is only taken for DMA_buf imports, which is something not
available for AGP either.
So that NULL pointer deref can't trigger either,
Christian.
>
> 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@xxxxxxxxxxxxxxxxxxxxx
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
