在 2018/11/24 2:10, Koenig, Christian 写道:
> Am 23.11.18 um 15:10 schrieb Zhou, David(ChunMing):
>> 在 2018/11/23 21:30, Koenig, Christian 写道:
>>> Am 23.11.18 um 14:27 schrieb Zhou, David(ChunMing):
>>>> 在 2018/11/22 19:25, Christian König 写道:
>>>>> Am 22.11.18 um 07:56 schrieb Sharma, Deepak:
>>>>>> when returned fence is not valid mostly due to userspace ignored
>>>>>> previous error causes NULL pointer dereference.
>>>>> Again, this is clearly incorrect. The my other mails on the
>>>>> earlier patch.
>>>> Sorry for I didn't get your history, but looks from the patch
>>>> itself, it is still a valid patch, isn't it?
>>> No, the semantic of amdgpu_ctx_get_fence() is that we return NULL
>>> when the fence is already signaled.
>>>
>>> So this patch could totally break userspace because it changes the
>>> behavior when we try to sync to an already signaled fence.
>> Ah, I got your meaning, how about attached patch?
> Yeah something like this, but I would just give the
> DRM_SYNCOBJ_CREATE_SIGNALED instead.
>
> I mean that's what this flag is good for isn't it?
Yeah, I give a flag initally when creating patch, but as you know, there is a
swich case not be able to use that flag:
case AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD:
fd = get_unused_fd_flags(O_CLOEXEC);
if (fd < 0) {
dma_fence_put(fence);
return fd;
}
sync_file = sync_file_create(fence);
dma_fence_put(fence);
if (!sync_file) {
put_unused_fd(fd);
return -ENOMEM;
}
fd_install(fd, sync_file->file);
info->out.handle = fd;
return 0;
So I change to stub fence instead.
-David
I have not applied this patch.
The issue was trying to address is when amdgpu_cs_ioctl() failed due to low
memory (ENOMEM) but userspace chose to proceed and called
amdgpu_cs_fence_to_handle_ioctl().
In amdgpu_cs_fence_to_handle_ioctl(), fence is null and later causing NULL
pointer dereference, this patch was to avoid that and system panic
But I understand now that its a valid case retuning NULL if fence was already
signaled but need to handle case so avoid kernel panic. Seems David patch
should fix this, I will test it tomorrow.
-Deepak
>
> Christian.
>
>> -David
>>> If that patch was applied then please revert it immediately.
>>>
>>> Christian.
>>>
>>>> -David
>>>>> If you have already pushed the patch then please revert.
>>>>>
>>>>> Christian.
>>>>>
>>>>>> Signed-off-by: Deepak Sharma <deepak.sha...@amd.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++
>>>>>> 1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>>> index 024dfbd87f11..14166cd8a12f 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>>> @@ -1403,6 +1403,8 @@ static struct dma_fence
>>>>>> *amdgpu_cs_get_fence(struct amdgpu_device *adev,
>>>>>> fence = amdgpu_ctx_get_fence(ctx, entity, user->seq_no);
>>>>>> amdgpu_ctx_put(ctx);
>>>>>> + if(!fence)
>>>>>> + return ERR_PTR(-EINVAL);
>>>>>> return fence;
>>>>>> }
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
