[PATCH] dma-buf/sw_sync: hold a fence reference when check if it signaled

[Chris Wilson]

Quoting Gustavo Padovan (2017-07-28 02:57:25)
> 2017-07-27 Chris Wilson <chris at chris-wilson.co.uk>:
> 
> > Quoting Gustavo Padovan (2017-07-27 20:03:53)
> > > From: Gustavo Padovan <gustavo.padovan at collabora.com>
> > > 
> > > If userspace already dropped its own reference by closing the sw_sync
> > > fence fd we might end up in a deadlock where
> > > dma_fence_is_signaled_locked() will trigger the release of the fence a
> > > thus try to hold the lock to remove the fence from the list.
> > 
> > So the issue here is that call to dma_fence_is_signaled_lock() is
> > triggering the unreference?
> 
> Exactly. I'll say that explicitely in the commit message.

:) It was more of a rhetorical question making sure that I understood
correctly.

> > But if I understand correctly, we just need to unlink first, then
> > signal.
> > 
> > list_for_each_entry_safe() {
> >       if (!timeline_fence_signaled(&pt->base))
> >               break;
> > 
> >       list_del_init(&pt->link);
> >       rb_erase(&pt->node, &obj->pt_tree);
> > 
> >       dma_fence_signal_locked(&pt->base);
> > }
> > 
> > The challenge is in writing the comment to explain the open-coding.
> 
> That is cleaner and doesn't need the get/put dance. I'll come up with a
> comment to explain it.

...

/*
 * A signal callback may release the last reference to this fence,
 * causing it to be freed. That operation has to be last to avoid
 * a use after free inside this loop, and must be after we remove
 * the fence from the timeline in order to prevent deadlocking on
 * timeline->lock inside timeline_fence_release().
 */
 dma_fence_signal_locked().
-Chris