[PATCH v2 0/2] iio: buffer-dmaengine: fix dma_vec building for coalesced sg tables

[PATCH v2 0/2] iio: buffer-dmaengine: fix dma_vec building for coalesced sg tables

[Nuno Sá]

Patch 1 fixes iio_dmaengine_buffer_submit_block() mixing up the CPU and the
DMA view of a DMABUF's scatterlist: it counted entries with
sg_nents_for_len() (CPU lengths) but consumed sg_dma_address()/sg_dma_len(),
which are only valid for the first sgt->nents entries. With an IOMMU
coalescing the mapping, the loop walks past the mapped set and hands a
garbage vec to the DMA engine, which wedges the buffer.

Patch 2 is the cleanup Jonathan spotted while reviewing v1: the max_size
computation at the top of the same function is dead, as only the fileio
branch consumes it and that branch computes it again where it is used.

---
Changes in v2:
- Patch 1:
  - Size the vec array with sgt->nents instead of sg_nents_for_dma().
  - Cut the new comment down to the invariant a reader needs.
  - Moved 'nents = i' up to right after the fill loop.
  - MOved `sgl = block->sg_table->sgl` to the place where we need sgl.
- Patch 2:
  - New patch dropping the dead max_size computation at the top of
    iio_dmaengine_buffer_submit_block() (Jonathan).
- Link to v1: https://patch.msgid.link/20260818-iio-buffer-dmabuf-iommu-fic-v1-1-4ff1e44a5073@analog.com

---
Michael Hennerich (1):
      iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

Nuno Sá (1):
      iio: buffer-dmaengine: drop dead max_size computation

 drivers/iio/buffer/industrialio-buffer-dmaengine.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
---
base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
--

Thanks!
- Nuno Sá

[PATCH v2 1/2] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Nuno Sá]

From: Michael Hennerich <michael.hennerich@analog.com>

iio_dmaengine_buffer_submit_block() counts scatterlist entries with
sg_nents_for_len(), which walks the CPU-side lengths (sg->length), but
then consumes the DMA-side fields (sg_dma_address()/sg_dma_len()).
After dma_map_sgtable() the two views may differ: an IOMMU can coalesce
the mapping so that only the first sgt->nents entries carry valid DMA
addresses, with nents < orig_nents.

On x86 with an IOMMU enabled, a DMABUF block backed by two 1 MiB
system-heap chunks maps to a single 2 MiB IOVA range. The CPU-side
count is 2, so the loop reads one entry past the mapped set and emits a
garbage vec ({addr = ~0, len = 0}). The DMA engine driver rejects the
vec array (prep returns NULL), the fence is signalled with -ENOMEM,
which a userspace poller cannot observe, and the block is left in
ACTIVE state so every further enqueue of it fails with -EBUSY. The
visible symptom is a stream of zero-filled blocks followed by a wedged
buffer.

Platforms without an IOMMU never hit this because nents == orig_nents.

Size the vec array with sgt->nents, i.e. the DMA-mapped view, and stop
the fill loop once bytes_used is covered - which is allowed to be
smaller than the block size - passing the number of vecs actually
filled to dmaengine_prep_peripheral_dma_vec().

One vec per mapped entry is enough since coalescing can only ever
reduce the number of entries. A single mapped entry longer than the
device's maximum segment size would need more than one, but the DMA API
already assumes no single segment exceeds it [1], and splitting a vec
down to the hardware descriptor size is the DMA engine driver's job -
which both current .device_prep_peripheral_dma_vec() implementations
do.

[1]: commit ab2cbeb0ed30 ("iommu/dma: Handle SG length overflow better")

Assisted-by: Claude:claude-fable-5
Fixes: 7a86d469983a ("iio: buffer-dmaengine: Support new DMABUF based userspace API")
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>

---

Note the Signed-off-by is just because I'm carrying Michael's patch!
---
 drivers/iio/buffer/industrialio-buffer-dmaengine.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index ecc02a427b92..376486be3f55 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -104,10 +104,13 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 	if (block->sg_table) {
 		unsigned long flags;
 
-		sgl = block->sg_table->sgl;
-		nents = sg_nents_for_len(sgl, block->bytes_used);
-		if (nents < 0)
-			return nents;
+		/*
+		 * Only the first sgt->nents entries carry a valid
+		 * sg_dma_address()/sg_dma_len() pair as mapping the table may
+		 * have coalesced entries, in which case nents is smaller than
+		 * orig_nents.
+		 */
+		nents = block->sg_table->nents;
 
 		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
 		if (!vecs)
@@ -115,7 +118,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 
 		len_total = block->bytes_used;
 
-		for (i = 0; i < nents; i++) {
+		sgl = block->sg_table->sgl;
+		for (i = 0; i < nents && len_total; i++) {
 			vecs[i].addr = sg_dma_address(sgl);
 			vecs[i].len = min(sg_dma_len(sgl), len_total);
 			len_total -= vecs[i].len;
@@ -123,6 +127,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 			sgl = sg_next(sgl);
 		}
 
+		nents = i;
+
 		if (block->cyclic)
 			flags = DMA_PREP_REPEAT;
 		else

-- 
2.55.0

[PATCH v2 2/2] iio: buffer-dmaengine: drop dead max_size computation

[Nuno Sá]

iio_dmaengine_buffer_submit_block() computes max_size unconditionally at
the top of the function, but only the fileio branch consumes it - and
that branch already computes it again right where it is used. Drop the
unconditional copy.

No functional change intended.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/buffer/industrialio-buffer-dmaengine.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 376486be3f55..284d4ef92104 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -88,9 +88,6 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 	unsigned int i;
 	int nents;
 
-	max_size = min(block->size, dmaengine_buffer->max_size);
-	max_size = round_down(max_size, dmaengine_buffer->align);
-
 	if (queue->buffer.direction == IIO_BUFFER_DIRECTION_IN)
 		dma_dir = DMA_DEV_TO_MEM;
 	else

-- 
2.55.0

Re: [PATCH v2 1/2] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Jonathan Cameron]

On Fri, 28 Aug 2026 11:46:55 +0100
Nuno Sá <nuno.sa@analog.com> wrote:

> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> iio_dmaengine_buffer_submit_block() counts scatterlist entries with
> sg_nents_for_len(), which walks the CPU-side lengths (sg->length), but
> then consumes the DMA-side fields (sg_dma_address()/sg_dma_len()).
> After dma_map_sgtable() the two views may differ: an IOMMU can coalesce
> the mapping so that only the first sgt->nents entries carry valid DMA
> addresses, with nents < orig_nents.
> 
> On x86 with an IOMMU enabled, a DMABUF block backed by two 1 MiB
> system-heap chunks maps to a single 2 MiB IOVA range. The CPU-side
> count is 2, so the loop reads one entry past the mapped set and emits a
> garbage vec ({addr = ~0, len = 0}). The DMA engine driver rejects the
> vec array (prep returns NULL), the fence is signalled with -ENOMEM,
> which a userspace poller cannot observe, and the block is left in
> ACTIVE state so every further enqueue of it fails with -EBUSY. The
> visible symptom is a stream of zero-filled blocks followed by a wedged
> buffer.
> 
> Platforms without an IOMMU never hit this because nents == orig_nents.
> 
> Size the vec array with sgt->nents, i.e. the DMA-mapped view, and stop
> the fill loop once bytes_used is covered - which is allowed to be
> smaller than the block size - passing the number of vecs actually
> filled to dmaengine_prep_peripheral_dma_vec().
> 
> One vec per mapped entry is enough since coalescing can only ever
> reduce the number of entries. A single mapped entry longer than the
> device's maximum segment size would need more than one, but the DMA API
> already assumes no single segment exceeds it [1], and splitting a vec
> down to the hardware descriptor size is the DMA engine driver's job -
> which both current .device_prep_peripheral_dma_vec() implementations
> do.
> 
> [1]: commit ab2cbeb0ed30 ("iommu/dma: Handle SG length overflow better")
> 
> Assisted-by: Claude:claude-fable-5
> Fixes: 7a86d469983a ("iio: buffer-dmaengine: Support new DMABUF based userspace API")
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Applied to the fixes-togreg branch of iio.git and marked for stable.

Seems I can get away with taking patch 2 via the testing branch as
well :)  So done that.

Both those branches will get rebased so if anyone else has feedback on this
series it would be good to have.

Thanks,

Jonathan

> 
> ---
> 
> Note the Signed-off-by is just because I'm carrying Michael's patch!
> ---
>  drivers/iio/buffer/industrialio-buffer-dmaengine.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> index ecc02a427b92..376486be3f55 100644
> --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> @@ -104,10 +104,13 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  	if (block->sg_table) {
>  		unsigned long flags;
>  
> -		sgl = block->sg_table->sgl;
> -		nents = sg_nents_for_len(sgl, block->bytes_used);
> -		if (nents < 0)
> -			return nents;
> +		/*
> +		 * Only the first sgt->nents entries carry a valid
> +		 * sg_dma_address()/sg_dma_len() pair as mapping the table may
> +		 * have coalesced entries, in which case nents is smaller than
> +		 * orig_nents.
> +		 */
> +		nents = block->sg_table->nents;
>  
>  		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
>  		if (!vecs)
> @@ -115,7 +118,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  
>  		len_total = block->bytes_used;
>  
> -		for (i = 0; i < nents; i++) {
> +		sgl = block->sg_table->sgl;
> +		for (i = 0; i < nents && len_total; i++) {
>  			vecs[i].addr = sg_dma_address(sgl);
>  			vecs[i].len = min(sg_dma_len(sgl), len_total);
>  			len_total -= vecs[i].len;
> @@ -123,6 +127,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  			sgl = sg_next(sgl);
>  		}
>  
> +		nents = i;
> +
>  		if (block->cyclic)
>  			flags = DMA_PREP_REPEAT;
>  		else
>