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

[Chen Jeffy]

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.