Re: [PATCH] iio: buffer: hold a reference while DMA-BUF is attached

["Nuno Sá" <nuno.sa@analog.com>]

On Wed, Nov 04, 2026 at 05:50:01PM +0800, Yaohan Li wrote:
> iio_buffer_attach_dmabuf() stores an IIO buffer pointer in the DMA-BUF
> attachment without taking a buffer reference. A delayed DMA fence
> completion can release the attachment after the IIO device has been
> unbound and its buffer freed. The release callback then dereferences
> the stale buffer while detaching the DMA-BUF.
> 
> Take a reference when storing the buffer in the attachment, and drop
> it after the final detach or when the attach operation fails. This
> keeps the buffer alive for the asynchronous completion path.
> 
> A KASAN-enabled test with a DMA-BUF-capable IIO test device reproduced
> the use-after-free on the unmodified kernel. The same delayed
> completion sequence produced no report with the change applied.
> 
> Fixes: 3e26d9f08fbe0 ("iio: core: Add new DMABUF interface infrastructure")
> Assisted-by: ZCode:GLM-4
> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
> ---

Yes, the change looks reasonable to me! Assuming the cdev is closed and
userspace detaches the block before the unbind we do force all dma to
terminate on unbind and the cleanup path does shedule_work() for
the fence and so iio_dev_release() might drop the last reference to the
buffer before the fence callback is called.

The whole thing is a bit hard to reason so unless someone has strong
evidence the weak reference can be used, I would very much take the
patch. Hence:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/industrialio-buffer.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 4294101810fd7..bfa6d19471126 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1597,6 +1597,8 @@ static void iio_buffer_dmabuf_release(struct kref *ref)
>  
>  	buffer->access->detach_dmabuf(buffer, priv->block);
>  
> +	iio_buffer_put(buffer);
> +
>  	dma_buf_detach(attach->dmabuf, attach);
>  	dma_buf_put(dmabuf);
>  	kfree(priv);
> @@ -1741,7 +1743,13 @@ static int iio_buffer_attach_dmabuf(struct iio_dev_buffer_pair *ib,
>  	}
>  
>  	kref_init(&priv->ref);
> -	priv->buffer = buffer;
> +	/*
> +	 * Hold a reference for the asynchronous dmabuf completion path
> +	 * (iio_buffer_cleanup() -> iio_buffer_dmabuf_put() may run from a
> +	 * work item after the device has been unbound); released in
> +	 * iio_buffer_dmabuf_release().
> +	 */
> +	priv->buffer = iio_buffer_get(buffer);
>  	priv->attach = attach;
>  	attach->importer_priv = priv;
>  
> @@ -1789,6 +1797,7 @@ static int iio_buffer_attach_dmabuf(struct iio_dev_buffer_pair *ib,
>  err_dmabuf_put:
>  	dma_buf_put(dmabuf);
>  err_free_priv:
> +	iio_buffer_put(priv->buffer);
>  	kfree(priv);
>  
>  	return err;
> -- 
> 2.34.1
>