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

[Alex Deucher]

On Wed, Jul 29, 2015 at 2:55 PM, Mario Kleiner
<mario.kleiner.de at gmail.com> wrote:
> On 07/29/2015 08:50 PM, Alex Deucher wrote:
>>
>> 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
>>
>
> Ah ok. Could you get (v2) instead? It has the same fix as Emil's to make
> sure we accept fd == 0 as a valid descriptor.

Sure.  Can you send it to me?  I can't seem to find it in my email.

Alex

>
> thanks,
> -mario
>
>
>
>
>
>
>
>>>
>>> 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