[PATCH] drm/gem: Internally test import_attach for imported objects

[Boris Brezillon]

On Tue, 15 Apr 2025 14:19:20 +0200
Christian König <christian.koenig at amd.com> wrote:

> Am 15.04.25 um 12:45 schrieb Thomas Zimmermann:
> > Hi
> >
> > Am 15.04.25 um 11:39 schrieb Christian König:  
> >> Am 15.04.25 um 11:20 schrieb Thomas Zimmermann:  
> >>> Test struct drm_gem_object.import_attach to detect imported
> >>> objects during cleanup. At that point, the imported DMA buffer
> >>> might have already been released and the dma_buf field is NULL.
> >>> The object's free callback then does a cleanup as for native
> >>> objects.  
> >> I don't think that this is a good idea.
> >>
> >> The DMA-buf is separately reference counted through the import
> >> attachment.  
> >
> > I understand that it's not ideal, but testing for import_attach to
> > be set is what we currently do throughout drivers. Putting this
> > behind an interface is already a step forward. 
> >>  
> >>> Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually
> >>> clear the dma_buf field in
> >>> drm_gem_object_exported_dma_buf_free().  
> >> That is for exported DMA-buf and should *NEVER* be used for
> >> imported ones.  
> >
> > Did you look at the discussion at the Closes tag? Where else could
> > dma-buf be cleared?  
> 
> Yeah, I've seen that. The solution is just completely wrong.
> 
> See for the export case obj->dma_buf points to the exported DMA-buf
> and causes a circle dependency when not set to NULL when the last
> handle is released.

I can confirm this is causing a regression in Panthor, where the driver
holds references to GEMs that might have been released by userspace, so
it's not just drivers calling drm_mode_destroy_dumb_ioctl(). This leads
drm_gem_is_imported() to return inconsistent values depending on when
the function is called (before/after the handle creation/destruction).
This patch seems to fix the problem, but I can't tell if it's the right
thing to do.