Re: [PATCH 01/15] iio: buffer: let the block keep its DMA fence

[Nuno Sá]

On Wed, Oct 28, 2026 at 09:30:19AM +0000, Nuno Sá wrote:
> On Tue, Oct 27, 2026 at 05:19:05PM +0100, Greg Kroah-Hartman wrote:
> > From: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > 
> > iio_buffer_enqueue_dmabuf() hands a plain pointer to the fence to the
> > buffer implementation, which keeps it in block->fence, and since the fix
> > for the fence leak it drops the initial reference right after
> > dma_resv_add_fence(). The only reference left is the one of the
> > reservation object.
> > 
> > That one does not last until the fence is signalled. dma_resv_add_fence()
> > replaces and puts a fence as soon as a later fence of the same context and
> > the same usage is added. All the fences of an attachment share a context,
> > and an enqueue on an output buffer only waits for the writers, not for the
> > read fence of the enqueue before it. A second IIO_BUFFER_DMABUF_ENQUEUE of
> > a buffer which is still queued is refused by iio_dma_can_enqueue_block(),
> > but by then its fence has taken the place of the first one. When the DMA
> > is done, iio_buffer_signal_dmabuf_done() signals a fence which has been
> > freed and schedules the work item in it.
> > 
> > The leak came from the reference which iio_buffer_signal_dmabuf_done()
> > takes and iio_buffer_cleanup() drops: with it nobody dropped the initial
> > one. Do not take it, and leave the initial reference to the block until
> > the cleanup.
> > 
> > Fixes: a09399935508 ("iio: buffer: Fix DMA fence leak in iio_buffer_enqueue_dmabuf()")
> > Cc: stable <stable@xxxxxxxxxx>
> > Cc: David Lechner <dlechner@xxxxxxxxxxxx>
> > Cc: "Nuno Sá" <nuno.sa@xxxxxxxxxx>
> > Cc: Andy Shevchenko <andy@xxxxxxxxxx>
> > Cc: "Benoît Monin" <benoit.monin@xxxxxxxxxxx>
> > Cc: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > ---
> 
> Patch looks good to me! Maybe Paul remembers why all of that refcouting
> dance was needed. Like this, it feels more natural:
> 
> Reviewed-by: Nuno Sá <nuno.sa@xxxxxxxxxx>

Also suspect we have the save problem in the USB side of things:

https://elixir.bootlin.com/linux/v7.3/source/drivers/usb/gadget/function/f_fs.c#L1741

- Nuno Sá

> 
> >  drivers/iio/industrialio-buffer.c | 15 +++++++--------
> >  1 file changed, 7 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> > index 4294101810fd..a2386dba497c 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -1931,9 +1931,14 @@ static int iio_buffer_enqueue_dmabuf(struct iio_dev_buffer_pair *ib,
> >  	if (ret)
> >  		goto err_queue_unlock;
> >  
> > +	/*
> > +	 * The reservation object takes a reference of its own, and drops it as
> > +	 * soon as a later fence of this attachment takes the place of this
> > +	 * one. The initial reference stays with the block, which points to the
> > +	 * fence until iio_buffer_cleanup() has run.
> > +	 */
> >  	dma_resv_add_fence(dmabuf->resv, &fence->base,
> >  			   dma_to_ram ? DMA_RESV_USAGE_WRITE : DMA_RESV_USAGE_READ);
> > -	dma_fence_put(&fence->base);
> >  	dma_resv_unlock(dmabuf->resv);
> >  
> >  	cookie = dma_fence_begin_signalling();
> > @@ -1989,18 +1994,12 @@ void iio_buffer_signal_dmabuf_done(struct dma_fence *fence, int ret)
> >  		container_of(fence, struct iio_dma_fence, base);
> >  	bool cookie = dma_fence_begin_signalling();
> >  
> > -	/*
> > -	 * Get a reference to the fence, so that it's not freed as soon as
> > -	 * it's signaled.
> > -	 */
> > -	dma_fence_get(fence);
> > -
> >  	fence->error = ret;
> >  	dma_fence_signal(fence);
> >  	dma_fence_end_signalling(cookie);
> >  
> >  	/*
> > -	 * The fence will be unref'd in iio_buffer_cleanup.
> > +	 * The reference of the block will be dropped in iio_buffer_cleanup.
> >  	 * It can't be done here, as the unref functions might try to lock the
> >  	 * resv object, which can deadlock.
> >  	 */
> > 
> > -- 
> > 2.56.0
> >