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

[Mario Kleiner]

On 07/29/2015 09:05 PM, Alex Deucher wrote:
> 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
>

Will do in a second.
-mario

>>
>> 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