Am 30.07.19 um 11:14 schrieb zhoucm1:
>
>
> On 2019年07月30日 17:04, Koenig, Christian wrote:
>> Am 30.07.19 um 10:47 schrieb 亿一:
>>> Hi alll,
>>> While analyzing the source code, I notice that function
>>> amdgpu_cs_process_fence_dep() may exist NULL pointer dereference and
>>> memory leak in the following code fragments:
>>>
>>>
>>> fence = amdgpu_ctx_get_fence(ctx, entity,
>>> deps[i].handle);
>>>
>>> if (chunk->chunk_id == AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES) {
>>> struct drm_sched_fence *s_fence = to_drm_sched_fence(fence);
>>> struct dma_fence *old = fence;
>>>
>>> fence = dma_fence_get(&s_fence->scheduled);
>>> dma_fence_put(old);
>>> }
>>>
>>> if (IS_ERR(fence)) {
>>> r = PTR_ERR(fence);
>>> amdgpu_ctx_put(ctx);
>>> return r;
>>> } else if (fence) {
>>> r = amdgpu_sync_fence(p->adev, &p->job->sync, fence,
>>> true);
>>> dma_fence_put(fence);
>>> amdgpu_ctx_put(ctx);
>>> if (r)
>>> return r;
>>> }
>>>
>>> function amdgpu_ctx_get_fence may return NULL pointer, which will
>>> cause NULL pointer dereference. What's more, IS_ERR() would not
>>> return true when pointer is NULL, which will cause the ctx reference
>>> leaked.
>> That handling is actually correct.
>>
>> The problem is the "if (chunk->chunk_id ==
>> AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES)" stuff above.
>>
>> That comes to early and needs to be moved below checking the fence for
>> errors. Going to send a fix for this to the mailing list in a minute.
> Lin Yi is right I think, we leaked ctx reference when fence is NULL.
Indeed, but what I meant was the a NULL fence here is not an error.
Just send out a patch to fix that stuff up, please review.
Christian.
>
> -David
>>
>> Thanks for the notice,
>> Christian.
>>
>>> But I don't know how to fix it, so report it to you all.
>>>
>>> Best Regards.
>>> Lin Yi.
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
