Re: [PATCH 1/2] drm/syncobj: fix fence and syncobj leak in drm_syncobj_import_sync_file_fence

[Julian Orth]

On Mon, Oct 12, 2026 at 9:45 AM Christian König
<christian.koenig@xxxxxxx> wrote:
>
>
>
> On 10/10/26 20:07, Julian Orth wrote:
> > If dma_fence_chain_alloc() fails, the function returns -ENOMEM without
> > dropping the references to the imported fence and the syncobj.
> >
> > Fixes: c2d3a7300695 ("drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs")
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Julian Orth <ju.orth@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/drm_syncobj.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> > index c23a5de27..5ca5163d0 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -730,6 +730,7 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
> >  {
> >       struct dma_fence *fence = sync_file_get_fence(fd);
> >       struct drm_syncobj *syncobj;
> > +     int ret = 0;
>
> Please don't initialize return values on declaration that is often considered bad coding style.
>
> >
> >       if (!fence)
> >               return -EINVAL;
> > @@ -743,17 +744,20 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
> >       if (point) {
>
> >               struct dma_fence_chain *chain = dma_fence_chain_alloc();
> >
> > -             if (!chain)
> > -                     return -ENOMEM;
> > +             if (!chain) {
> > +                     ret = -ENOMEM;
> > +                     goto out;
> > +             }
> >
> >               drm_syncobj_add_point(syncobj, chain, fence, point);
>
> Allocation and adding the point is intentionally separated so that dma_fence_chain_alloc() can be called before looking up the syncobj or previous fence.
>
> I suggest to make use of that.

This is the fourth time that someone has tried to fix this trivial bug
for over a year. The previous 3 cases were:

https://lore.kernel.org/all/c715ef30-9d91-46f3-8a0f-1039f3d25e7d@stanley.mountain/
https://lore.kernel.org/all/20260516-jorth-syncobj-v1-9-88ede9d98a81@xxxxxxxxx/
https://lore.kernel.org/all/20260525025455.3887523-1-vulab@xxxxxxxxxxx/

The last of which didn't even receive a reply. And there have been
more cases in the past where I encountered a dri-devel-adjacent issue,
googled it, and found that patches had already been sent in years ago
to fix it, but were ignored.

Which is all to explain why I had all of the work for this series done by AI.

I heard some chatter about dri-devel adopting a gitlab-based workflow,
and I hope that works out and I didn't misunderstand. Because the
current system is clearly broken. Nobody keeps track of which bugs
have been reported, nobody triages existing bugs, nobody keeps track
of attempts to fix bugs. Which is how 3 people ended up doing repeat
work of discovering, analyzing, and fixing the same bug.

The same pattern applies to my patch two. I clearly don't know enough
about the (probably) undocumented semantics of where and when which
structure of fences can be used. Which is why I sent in a bug report
about this issue in May instead of trying to fix it on my own:
https://lore.kernel.org/all/CAHijbEUKuXNeSnT82P-ojOhzU5siNHjxUW+qEDOqx07Ys9m=xw@xxxxxxxxxxxxxx/

I would have hoped that a bug report amounting to "this new feature
causes the kernel to spam dmesg at 60 fps" would get some attention.
But no. The only way to get this issue fixed seems to be me acting as
a meat proxy to fix an issue that I know very little about.

Thanks for reading my blog post.


>
> Regards,
> Christian.
>
> >       } else {
> >               drm_syncobj_replace_fence(syncobj, fence);
> >       }
> >
> > +out:
> >       dma_fence_put(fence);
> >       drm_syncobj_put(syncobj);
> > -     return 0;
> > +     return ret;
> >  }
> >
> >  static int drm_syncobj_export_sync_file(struct drm_file *file_private,
>