[PATCH 1/3] iio: buffer: Fix potential use-after-free in anonymous buffer release

[PATCH 1/3] iio: buffer: Fix potential use-after-free in anonymous buffer release

[Lars-Peter Clausen]

An anonymous buffer handle holds a reference to the underlying IIO device.
The reference is dropped in the buffer handle's release function. If the
device has been removed, either through unbind or hot-unplug, the buffer
handle might hold the last reference.

The release function takes the mutex for the buffer using a guard, which
means the unlock happens after all the code in the function, including
`iio_device_put()`. If the anonymous buffer holds the last reference this
might free both the IIO device and the buffer, which contains the mutex,
leading to use-after-free when the mutex is unlocked.

Fix this by using a scoped guard just around the buffer dmabuf list access,
making sure the mutex is unlocked before releasing the IIO device.

Version 10 of the patch that introduced this issue used this exact scheme
of first unlocking and then dropping the reference [1]. During review it
was suggested to use a guard instead, and version 11 made that change [2].

[1] https://lore.kernel.org/linux-iio/20240605110845.86740-4-paul@crapouillou.net
[2] https://lore.kernel.org/linux-iio/20240618100302.72886-4-paul@crapouillou.net

Reported-by: codex:gpt-5.6
Fixes: 3e26d9f08fbe ("iio: core: Add new DMABUF interface infrastructure")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 531fc4ccc15de..04b3916f89e2d 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1619,12 +1619,16 @@ static int iio_buffer_chrdev_release(struct inode *inode, struct file *filep)
 
 	wake_up(&buffer->pollq);
 
-	guard(mutex)(&buffer->dmabufs_mutex);
-
-	/* Close all attached DMABUFs */
-	list_for_each_entry_safe(priv, tmp, &buffer->dmabufs, entry) {
-		list_del_init(&priv->entry);
-		iio_buffer_dmabuf_put(priv->attach);
+	/*
+	 * The mutex must be unlocked before iio_device_put(), which might drop the
+	 * last reference and free the buffer.
+	 */
+	scoped_guard(mutex, &buffer->dmabufs_mutex) {
+		/* Close all attached DMABUFs */
+		list_for_each_entry_safe(priv, tmp, &buffer->dmabufs, entry) {
+			list_del_init(&priv->entry);
+			iio_buffer_dmabuf_put(priv->attach);
+		}
 	}
 
 	kfree(ib);
-- 
2.47.3

[PATCH 2/3] iio: buffer: Tie IIO dma fence lock lifetime to the fence

[Lars-Peter Clausen]

The `iio_dma_fence` implementation currently uses a lock embedded in the
`iio_dmabuf_priv`. But the `iio_dma_fence` can outlive the
`iio_dmabuf_priv`, which can cause a use-after-free.

Tie the lifetime of the lock to the lifetime of the fence by embedding them
in the same struct.

We can't just hold a reference to the `iio_dmabuf_priv` from the
`iio_dma_fence` since `iio_buffer_dmabuf_release()` might sleep and the
fence release callback is not allowed to sleep.

Note that the `dma_fence` framework now has an internal lock that gets used
when the passing `NULL` for `lock` in `dma_fence_init()`, but in order to
allow this patch to be backportable use an external lock.

Reported-by: codex:gpt-5.6
Fixes: 3e26d9f08fbe ("iio: core: Add new DMABUF interface infrastructure")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 04b3916f89e2d..31c06bffdce8a 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -47,9 +47,6 @@ struct iio_dmabuf_priv {
 
 	u64 context;
 
-	/* Spinlock used for locking the dma_fence */
-	spinlock_t lock;
-
 	struct dma_buf_attachment *attach;
 	struct sg_table *sgt;
 	enum dma_data_direction dir;
@@ -58,6 +55,7 @@ struct iio_dmabuf_priv {
 
 struct iio_dma_fence {
 	struct dma_fence base;
+	spinlock_t lock; /* protects base */
 	struct iio_dmabuf_priv *priv;
 	struct work_struct work;
 };
@@ -1706,7 +1704,6 @@ static int iio_buffer_attach_dmabuf(struct iio_dev_buffer_pair *ib,
 	if (!priv)
 		return -ENOMEM;
 
-	spin_lock_init(&priv->lock);
 	priv->context = dma_fence_context_alloc(1);
 
 	dmabuf = dma_buf_get(fd);
@@ -1896,6 +1893,8 @@ static int iio_buffer_enqueue_dmabuf(struct iio_dev_buffer_pair *ib,
 		goto err_attachment_put;
 	}
 
+	spin_lock_init(&fence->lock);
+
 	fence->priv = priv;
 
 	seqno = atomic_add_return(1, &priv->seqno);
@@ -1906,7 +1905,7 @@ static int iio_buffer_enqueue_dmabuf(struct iio_dev_buffer_pair *ib,
 	 * the dma_fence.
 	 */
 	dma_fence_init(&fence->base, &iio_buffer_dma_fence_ops,
-		       &priv->lock, priv->context, seqno);
+		       &fence->lock, priv->context, seqno);
 
 	ret = iio_dma_resv_lock(dmabuf, nonblock);
 	if (ret)
-- 
2.47.3

[PATCH 3/3] iio: buffer: Make IIO DMA fence release RCU-safe

[Lars-Peter Clausen]

The `dma_fence` documentation states that if a custom release
implementation is provided, the `dma_fence` object must be freed in an
RCU-safe way. The current `iio_dma_fence` implementation uses `kfree()`,
which might result in a use-after-free.

Remove the custom `release` implementation. This makes the DMA fence core
fall back to `dma_fence_free()`, which calls `kfree_rcu()` on the fence.
This requires that the fence be the first member of `struct iio_dma_fence`.

Using the default release method for extended DMA fence structures is a
common pattern.

Reported-by: codex:gpt-5.6
Fixes: 3e26d9f08fbe ("iio: core: Add new DMABUF interface infrastructure")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 31c06bffdce8a..2c9ec93dff475 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -54,6 +54,10 @@ struct iio_dmabuf_priv {
 };
 
 struct iio_dma_fence {
+	/*
+	 * Must remain the first member so the default release callback can pass
+	 * the fence directly to dma_fence_free().
+	 */
 	struct dma_fence base;
 	spinlock_t lock; /* protects base */
 	struct iio_dmabuf_priv *priv;
@@ -1828,18 +1832,9 @@ iio_buffer_dma_fence_get_driver_name(struct dma_fence *fence)
 	return "iio";
 }
 
-static void iio_buffer_dma_fence_release(struct dma_fence *fence)
-{
-	struct iio_dma_fence *iio_fence =
-		container_of(fence, struct iio_dma_fence, base);
-
-	kfree(iio_fence);
-}
-
 static const struct dma_fence_ops iio_buffer_dma_fence_ops = {
 	.get_driver_name	= iio_buffer_dma_fence_get_driver_name,
 	.get_timeline_name	= iio_buffer_dma_fence_get_driver_name,
-	.release		= iio_buffer_dma_fence_release,
 };
 
 static int iio_buffer_enqueue_dmabuf(struct iio_dev_buffer_pair *ib,
-- 
2.47.3

Re: [PATCH 3/3] iio: buffer: Make IIO DMA fence release RCU-safe

[Andy Shevchenko]

On Wed, Jul 15, 2026 at 08:42:45AM -0700, Lars-Peter Clausen wrote:
> The `dma_fence` documentation states that if a custom release
> implementation is provided, the `dma_fence` object must be freed in an
> RCU-safe way. The current `iio_dma_fence` implementation uses `kfree()`,
> which might result in a use-after-free.
> 
> Remove the custom `release` implementation. This makes the DMA fence core
> fall back to `dma_fence_free()`, which calls `kfree_rcu()` on the fence.
> This requires that the fence be the first member of `struct iio_dma_fence`.
> 
> Using the default release method for extended DMA fence structures is a
> common pattern.

...

> +	/*
> +	 * Must remain the first member so the default release callback can pass
> +	 * the fence directly to dma_fence_free().
> +	 */
>  	struct dma_fence base;

TBH, I don't like this trick. container_of() is there for a reason. Even if
it's the first member in the structure. Can we simply switch to RCU version?
Would it require big changes?

> -static void iio_buffer_dma_fence_release(struct dma_fence *fence)
> -{
> -	struct iio_dma_fence *iio_fence =
> -		container_of(fence, struct iio_dma_fence, base);
> -
> -	kfree(iio_fence);

So, we can't use dma_fence_free() here, because without above guarantee it will
get a wrong address. Maybe (maybe!) as a quick fix this is fine, but in long
term can we also have a refactoring patch that drops above comment at the end?

> -}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: buffer: Make IIO DMA fence release RCU-safe

[Lars-Peter Clausen]

On 7/15/26 9:17 AM, Andy Shevchenko wrote:
> On Wed, Jul 15, 2026 at 08:42:45AM -0700, Lars-Peter Clausen wrote:
>> The `dma_fence` documentation states that if a custom release
>> implementation is provided, the `dma_fence` object must be freed in an
>> RCU-safe way. The current `iio_dma_fence` implementation uses `kfree()`,
>> which might result in a use-after-free.
>>
>> Remove the custom `release` implementation. This makes the DMA fence core
>> fall back to `dma_fence_free()`, which calls `kfree_rcu()` on the fence.
>> This requires that the fence be the first member of `struct iio_dma_fence`.
>>
>> Using the default release method for extended DMA fence structures is a
>> common pattern.
> ...
>
>> +	/*
>> +	 * Must remain the first member so the default release callback can pass
>> +	 * the fence directly to dma_fence_free().
>> +	 */
>>   	struct dma_fence base;
> TBH, I don't like this trick. container_of() is there for a reason. Even if
> it's the first member in the structure. Can we simply switch to RCU version?
> Would it require big changes?
>
>> -static void iio_buffer_dma_fence_release(struct dma_fence *fence)
>> -{
>> -	struct iio_dma_fence *iio_fence =
>> -		container_of(fence, struct iio_dma_fence, base);
>> -
>> -	kfree(iio_fence);
> So, we can't use dma_fence_free() here, because without above guarantee it will
> get a wrong address. Maybe (maybe!) as a quick fix this is fine, but in long
> term can we also have a refactoring patch that drops above comment at the end?

I also think the container_of version is cleaner, but I checked all the 
other implementations and 18 out of the 20 dma_fence implementations use 
this same schema of just letting the default release handle it even when 
embedding the struct, so it seems to be the consensus that this is the 
way to go.

Re: [PATCH 3/3] iio: buffer: Make IIO DMA fence release RCU-safe

[Jonathan Cameron]

On Wed, 15 Jul 2026 10:21:19 -0700
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 7/15/26 9:17 AM, Andy Shevchenko wrote:
> > On Wed, Jul 15, 2026 at 08:42:45AM -0700, Lars-Peter Clausen wrote:  
> >> The `dma_fence` documentation states that if a custom release
> >> implementation is provided, the `dma_fence` object must be freed in an
> >> RCU-safe way. The current `iio_dma_fence` implementation uses `kfree()`,
> >> which might result in a use-after-free.
> >>
> >> Remove the custom `release` implementation. This makes the DMA fence core
> >> fall back to `dma_fence_free()`, which calls `kfree_rcu()` on the fence.
> >> This requires that the fence be the first member of `struct iio_dma_fence`.
> >>
> >> Using the default release method for extended DMA fence structures is a
> >> common pattern.  
> > ...
> >  
> >> +	/*
> >> +	 * Must remain the first member so the default release callback can pass
> >> +	 * the fence directly to dma_fence_free().
> >> +	 */
> >>   	struct dma_fence base;  
> > TBH, I don't like this trick. container_of() is there for a reason. Even if
> > it's the first member in the structure. Can we simply switch to RCU version?
> > Would it require big changes?
> >  
> >> -static void iio_buffer_dma_fence_release(struct dma_fence *fence)
> >> -{
> >> -	struct iio_dma_fence *iio_fence =
> >> -		container_of(fence, struct iio_dma_fence, base);
> >> -
> >> -	kfree(iio_fence);  
> > So, we can't use dma_fence_free() here, because without above guarantee it will
> > get a wrong address. Maybe (maybe!) as a quick fix this is fine, but in long
> > term can we also have a refactoring patch that drops above comment at the end?  
> 
> I also think the container_of version is cleaner, but I checked all the 
> other implementations and 18 out of the 20 dma_fence implementations use 
> this same schema of just letting the default release handle it even when 
> embedding the struct, so it seems to be the consensus that this is the 
> way to go.
> 
Another option might be to enforce it creation time. There are some
examples of this pattern like fwctl_alloc_device().  That lets all callbacks
associated with DMA fences know it is safe to assume it is the first element
but you do have to ensure everyone allocates their fence as something like

struct my_fence *fence = dma_fence_alloc(struct my_fence, dma_fence_member_name_in_my_fence);

Given this is a fix I don't mind taking current form and assuming we might
revisit the safety of this in future.

With that in mind I've queued up all 3 patches in the fixes-togreg branch
(tweaked patch 1 for link tags as suggested).  These are a little subtle
though so I'd appreciate anyone else who has time taking a look.

Jonathan

Re: [PATCH 1/3] iio: buffer: Fix potential use-after-free in anonymous buffer release

[Andy Shevchenko]

On Wed, Jul 15, 2026 at 08:42:43AM -0700, Lars-Peter Clausen wrote:
> An anonymous buffer handle holds a reference to the underlying IIO device.
> The reference is dropped in the buffer handle's release function. If the
> device has been removed, either through unbind or hot-unplug, the buffer
> handle might hold the last reference.
> 
> The release function takes the mutex for the buffer using a guard, which
> means the unlock happens after all the code in the function, including
> `iio_device_put()`. If the anonymous buffer holds the last reference this
> might free both the IIO device and the buffer, which contains the mutex,
> leading to use-after-free when the mutex is unlocked.
> 
> Fix this by using a scoped guard just around the buffer dmabuf list access,
> making sure the mutex is unlocked before releasing the IIO device.
> 
> Version 10 of the patch that introduced this issue used this exact scheme
> of first unlocking and then dropping the reference [1]. During review it
> was suggested to use a guard instead, and version 11 made that change [2].

> [1] https://lore.kernel.org/linux-iio/20240605110845.86740-4-paul@crapouillou.net
> [2] https://lore.kernel.org/linux-iio/20240618100302.72886-4-paul@crapouillou.net

Make them Link tags?

Link: ... [1]
Link: ... [2]

> Reported-by: codex:gpt-5.6
> Fixes: 3e26d9f08fbe ("iio: core: Add new DMABUF interface infrastructure")

The case and fix makes sense to me.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko