Re: [Linaro-mm-sig] Re: [PATCH v2] drm/gem: Fix GEM handle release errors

[Chen Jeffy <jeffy.chen@rock-chips.com>]

Hi Christian,

On 8/9 星期二 17:08, Christian König wrote:
> Hi Jeffy,
> 
> Am 09.08.22 um 09:55 schrieb Christian König:
>> [SNIP]
>>>>
>>>>>
>>>>>
>>>>> So we are allowing GEM object to have multiple handles, and GEM 
>>>>> object could have at most one dma-buf, doesn't that means that 
>>>>> dma-buf could map to multiple handles?
>>>>
>>>> No, at least not for the same GEM file private. That's the reason 
>>>> why the rb is indexed by the dma_buf object and not the handle.
>>>>
>>>> In other words the rb is so that you have exactly one handle for 
>>>> each dma_buf in each file private.
>>>
>>> I don't think so, because if user get multiple handles for the same 
>>> GEM obj and use drm_gem_prime_handle_to_fd() for those handles
>>
>> Mhm, that works? This is illegal and should have been prevented somehow.
> 
> At least I see the problem now. I'm just not sure how to fix it.
> 
> Your v2 patch indeed prevents leakage of the drm_prime_member for the 
> additional handles, but those shouldn't have been added in the first place.
> 
> The issue is that with this we make it unpredictable which handle is 
> returned. E.g. if we have handle 2,5,7 it can be that because of 
> re-balancing the tree sometimes 2 and sometimes 5 is returned.

Maybe cache the latest returned handle in the obj(after 
drm_gem_prime_fd_to_handle), and clear it when that handle been deleted 
in drm_gem_handle_delete()?

Something like:
drm_gem_prime_fd_to_handle
   handle = drm_prime_lookup_buf_handle(buf)
   obj = obj_from_handle(handle)
   if !obj->primary_handle
     obj->primary_handle = handle
   return obj->primary_handle

Or maybe limit GEM obj with a single lifetime handle?

> 
> That's not really a good idea and breaks a couple of assumptions as far 
> as I know.
> 
> Ideas?
> 
> Thanks,
> Christian.
>