mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] iio: buffer: hold a reference while DMA-BUF is attached
@ 2026-11-04  9:50 Yaohan Li
  2026-11-07 22:25 ` Jonathan Cameron
  2026-11-09 17:18 ` Nuno Sá
  0 siblings, 2 replies; 3+ messages in thread
From: Yaohan Li @ 2026-11-04  9:50 UTC (permalink / raw)
  To: jic23; +Cc: dlechner, nuno.sa, andy, paul, linux-iio, linux-kernel

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>
---
 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iio: buffer: hold a reference while DMA-BUF is attached
  2026-11-04  9:50 [PATCH] iio: buffer: hold a reference while DMA-BUF is attached Yaohan Li
@ 2026-11-07 22:25 ` Jonathan Cameron
  2026-11-09 17:18 ` Nuno Sá
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2026-11-07 22:25 UTC (permalink / raw)
  To: Yaohan Li; +Cc: dlechner, nuno.sa, andy, paul, linux-iio, linux-kernel

On Wed, 04 Nov 2026 17:50:01 +0800
Yaohan Li <shunsheng1218@gmail.com> 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

The guidance on these tags change recently. Now the preference is
for a brief explanation of how the LLM was used under the ---

This seems reasonable to me but there are others who know the
IIO dma buffer stuff better that I do, so I'll wait for them
to take a look

Thanks,

Jonathan

> Signed-off-by: Yaohan Li <shunsheng1218@gmail.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;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iio: buffer: hold a reference while DMA-BUF is attached
  2026-11-04  9:50 [PATCH] iio: buffer: hold a reference while DMA-BUF is attached Yaohan Li
  2026-11-07 22:25 ` Jonathan Cameron
@ 2026-11-09 17:18 ` Nuno Sá
  1 sibling, 0 replies; 3+ messages in thread
From: Nuno Sá @ 2026-11-09 17:18 UTC (permalink / raw)
  To: Yaohan Li; +Cc: jic23, dlechner, andy, paul, linux-iio, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-09 17:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-04  9:50 [PATCH] iio: buffer: hold a reference while DMA-BUF is attached Yaohan Li
2026-11-07 22:25 ` Jonathan Cameron
2026-11-09 17:18 ` Nuno Sá

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®