[Mesa-dev] [PATCH] winsys/radeon: don't leak the fd when it is 0

[Alex Deucher]

On Wed, Jul 29, 2015 at 2:48 PM, Mario Kleiner
<mario.kleiner.de at gmail.com> wrote:
>
>
> On 07/29/2015 05:46 PM, Alex Deucher wrote:
>>
>> On Wed, Jul 29, 2015 at 10:44 AM, Emil Velikov <emil.l.velikov at gmail.com>
>> wrote:
>>>
>>> Earlier commit added an extra dup(fd) to fix a ZaphodHeads issue.
>>> Although it did not consider the (very unlikely) case where we might end
>>> up with the valid fd == 0.
>>>
>>> Fixes: 28dda47ae4d(winsys/radeon: Use dup fd as key in drm-winsys hash
>>> table to fix ZaphodHeads.)
>>>
>>> Cc: Mario Kleiner <mario.kleiner.de at gmail.com>
>>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>>
>>
>> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
>>
>
> Looks good, thanks for catching this.
>
> Reviewed-by: Mario Kleiner <mario.kleiner.de at gmail.com>
>
> cc stable seems safe, so can't hurt, no?
>
> I have a similar patch for the amdgpu Zaphodheads fix floating on the list.
> v1 (just the dup(fd) fix, but not checking for fd >= 0) already reviewed,
> but v2 fixes the same problem on amdgpu you just fixed for radeon, but so
> far lacks a reviewed by - and i don't know who can push stuff to libdrm?
>
> The patch was "[PATCH] libdrm/amdgpu: Use private fd for amdgpu_device and
> winsys hash table to fix ZaphodHeads. (v2)"
>
> Maybe you can have a quick look to get all these into the next mesa release?

I already picked it up in my libdrm tree:
http://cgit.freedesktop.org/~agd5f/drm/commit/?h=amdgpu&id=e2b18a1300313912aeaecf5ba8dd896b5853f6d4

Alex

>
> thanks,
> -mario
>
>>> ---
>>>
>>> Perhaps we should CC mesa-stable as the offending commit, yet
>>> considering how unlikely the above case is I'm ambivalent,
>>>
>>> -Emil
>>>
>>>   src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
>>> b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
>>> index 41f8826..f7784fb 100644
>>> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
>>> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
>>> @@ -493,7 +493,7 @@ static void radeon_winsys_destroy(struct
>>> radeon_winsys *rws)
>>>           radeon_surface_manager_free(ws->surf_man);
>>>       }
>>>
>>> -    if (ws->fd)
>>> +    if (ws->fd >= 0)
>>>           close(ws->fd);
>>>
>>>       FREE(rws);
>>> @@ -786,7 +786,7 @@ fail:
>>>           ws->kman->destroy(ws->kman);
>>>       if (ws->surf_man)
>>>           radeon_surface_manager_free(ws->surf_man);
>>> -    if (ws->fd)
>>> +    if (ws->fd >= 0)
>>>           close(ws->fd);
>>>
>>>       FREE(ws);
>>> --
>>> 2.4.5
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev