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

[Mario Kleiner]

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.

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