[PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[PATCH] 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 sg_nents_for_dma(), which walks the DMA-mapped
view and accounts for max-length splitting, and stop the fill loop once
bytes_used is covered - which is allowed to be smaller than the block
size - passing the reduced count to dmaengine_prep_peripheral_dma_vec().

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, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index ecc02a427b92..bece45381c8c 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -104,10 +104,16 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 	if (block->sg_table) {
 		unsigned long flags;
 
+		/*
+		 * Use the DMA-mapped view of the sg_table: after mapping
+		 * (e.g. through an IOMMU) the DMA entries (sgt->nents) can be
+		 * fewer than the CPU entries, and sg_dma_address()/sg_dma_len()
+		 * are only valid for the first sgt->nents entries. Counting
+		 * with sg_nents_for_len() (CPU lengths) walks past them and
+		 * hands garbage vecs to the DMA engine.
+		 */
 		sgl = block->sg_table->sgl;
-		nents = sg_nents_for_len(sgl, block->bytes_used);
-		if (nents < 0)
-			return nents;
+		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);
 
 		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
 		if (!vecs)
@@ -115,7 +121,7 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 
 		len_total = block->bytes_used;
 
-		for (i = 0; i < nents; i++) {
+		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;
@@ -133,6 +139,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 		 * before it can run, so always set the EOT flag.
 		 */
 		flags |= DMA_PREP_LOAD_EOT;
+		nents = i;
+
 		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
 							 vecs, nents, dma_dir,
 							 flags);

---
base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
--

Thanks!
- Nuno Sá

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

[Jonathan Cameron]

On Tue, 18 Aug 2026 17:45:29 +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 sg_nents_for_dma(), which walks the DMA-mapped
> view and accounts for max-length splitting, and stop the fill loop once
> bytes_used is covered - which is allowed to be smaller than the block
> size - passing the reduced count to dmaengine_prep_peripheral_dma_vec().
> 
> 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>

There are some gremlins nearby in this code...
In the else just of this context seems max_size is computed again
having been done just above the code seen here.

Unless I'm missing something that should be cleaned up as well.

Been a while since I got my head into the scatterlist
stuff, so I might have it wrong below, but I don't think what
you have here actually works if the merging of entries is
larger than the max dma entry the hardware supports.


> ---
> 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, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> index ecc02a427b92..bece45381c8c 100644
> --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> @@ -104,10 +104,16 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  	if (block->sg_table) {
>  		unsigned long flags;
>  
> +		/*
> +		 * Use the DMA-mapped view of the sg_table: after mapping
> +		 * (e.g. through an IOMMU) the DMA entries (sgt->nents) can be
> +		 * fewer than the CPU entries, and sg_dma_address()/sg_dma_len()
> +		 * are only valid for the first sgt->nents entries. Counting
> +		 * with sg_nents_for_len() (CPU lengths) walks past them and
> +		 * hands garbage vecs to the DMA engine.

This feels like too much info after the fix is in place.  Talking about other
stuff that would be wrong is rather unusual.

> +		 */
>  		sgl = block->sg_table->sgl;
> -		nents = sg_nents_for_len(sgl, block->bytes_used);
> -		if (nents < 0)
> -			return nents;
> +		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);

So this fun function will generally give us the number of sgl entries, but not
quite always. It will give us how many chunks of up to max_size fit into
a particularly large entry.

>  
>  		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
>  		if (!vecs)
> @@ -115,7 +121,7 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  
>  		len_total = block->bytes_used;
>  
> -		for (i = 0; i < nents; i++) {
> +		for (i = 0; i < nents && len_total; i++) {
So this needs to be more clever as we aren't just iterating entrees and filling
them in, some of them could at least in theory be too big to fit
in a single vec - hence you need to do a loop in here that sets
multiple entries if that occurs.

If that can't happen for some other reason then I think you can 
just use block->sgtable->nents instead of the more complex call above.

>  			vecs[i].addr = sg_dma_address(sgl);
>  			vecs[i].len = min(sg_dma_len(sgl), len_total);
>  			len_total -= vecs[i].len;
> @@ -133,6 +139,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  		 * before it can run, so always set the EOT flag.
>  		 */
>  		flags |= DMA_PREP_LOAD_EOT;
> +		nents = i;
> +
>  		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
>  							 vecs, nents, dma_dir,
>  							 flags);
> 
> ---
> base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
> change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
> --
> 
> Thanks!
> - Nuno Sá
> 

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

[Nuno Sá]

On Wed, Aug 19, 2026 at 01:41:04AM +0100, Jonathan Cameron wrote:
> On Tue, 18 Aug 2026 17:45:29 +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 sg_nents_for_dma(), which walks the DMA-mapped
> > view and accounts for max-length splitting, and stop the fill loop once
> > bytes_used is covered - which is allowed to be smaller than the block
> > size - passing the reduced count to dmaengine_prep_peripheral_dma_vec().
> > 
> > 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>
> 
> There are some gremlins nearby in this code...
> In the else just of this context seems max_size is computed again
> having been done just above the code seen here.
> 
> Unless I'm missing something that should be cleaned up as well.

Oh yes! Something I already noticed a couple of times but never sent the
patch right away so I kept forgetting about it.
> 
> Been a while since I got my head into the scatterlist
> stuff, so I might have it wrong below, but I don't think what
> you have here actually works if the merging of entries is
> larger than the max dma entry the hardware supports.
> 

In theory might not be a bug but I'm not sure it's a claim we can fully
take as guarantee.

> 
> > ---
> > 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, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > index ecc02a427b92..bece45381c8c 100644
> > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > @@ -104,10 +104,16 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> >  	if (block->sg_table) {
> >  		unsigned long flags;
> >  
> > +		/*
> > +		 * Use the DMA-mapped view of the sg_table: after mapping
> > +		 * (e.g. through an IOMMU) the DMA entries (sgt->nents) can be
> > +		 * fewer than the CPU entries, and sg_dma_address()/sg_dma_len()
> > +		 * are only valid for the first sgt->nents entries. Counting
> > +		 * with sg_nents_for_len() (CPU lengths) walks past them and
> > +		 * hands garbage vecs to the DMA engine.
> 
> This feels like too much info after the fix is in place.  Talking about other
> stuff that would be wrong is rather unusual.
>

Oh well, I guess LLM over commenting as usual.

> > +		 */
> >  		sgl = block->sg_table->sgl;
> > -		nents = sg_nents_for_len(sgl, block->bytes_used);
> > -		if (nents < 0)
> > -			return nents;
> > +		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);
> 
> So this fun function will generally give us the number of sgl entries, but not
> quite always. It will give us how many chunks of up to max_size fit into
> a particularly large entry.
> 
> >  
> >  		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
> >  		if (!vecs)
> > @@ -115,7 +121,7 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> >  
> >  		len_total = block->bytes_used;
> >  
> > -		for (i = 0; i < nents; i++) {
> > +		for (i = 0; i < nents && len_total; i++) {
> So this needs to be more clever as we aren't just iterating entrees and filling
> them in, some of them could at least in theory be too big to fit
> in a single vec - hence you need to do a loop in here that sets
> multiple entries if that occurs.

I see! But I think that merging means that we end up with more nents
(sg_nents_for_len()) entries than DMA ones which was the issue we had
because we were left with vecs with invalid addresses and 0 sized. The other
way around should not happen on the IOMMU path at least given the assumption
that a single sg  entry must not be bigger than max_len [1]. I think in theory
that can actually happen (if we end using a dma provider defaulting to 64K and 
cma dma_bufs) but maybe we can argue that's not our problem? Given
the assumptions of course.

But even if we somehow reach this path with sg_dma_address > max_size
and end up stuffing all of it in say, vec[0], the DMA controller should
also care to make sure it splits each vec if it exceeds the descriptor
max size. That's what both users of (ADI axi_dmac being one them)
.device_prep_peripheral_dma_vec() are doing today. But yes, I agree that
might be a bold assumption.

> 
> If that can't happen for some other reason then I think you can 
> just use block->sgtable->nents instead of the more complex call above.
> 

I was the one suggesting the other call because it seemed what we
actually wanted but I'm fine with just using the above. I'm more tempted
to keep it simple and use 'block->sgtable->nents' rather than jumping in
a more complex subloop without clear evidence we really need it.
Thoughts?

As a fun side note this also made visible another subtle issue. Given
that submitting a block might only happen when enabling the buffer (so
async to enqueueing it) when we hit this issue, we get -ENOMEM and
and go ahead  to wake up poll(). But AFAIK, we can't really propagate
the error code up to userspace so userspace wakes up just to get
garbage and thinking everything is fine. Easy way out would be to treat
this as and hard error and return error when enabling the buffer. But
that also raises the question that other blocks might have been properly
submitted though. Not really sure how to handle this but anyways a problem
for another day :)

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ab2cbeb0ed301a9f0460078e91b09f39958212ef

- Nuno Sá

> >  			vecs[i].addr = sg_dma_address(sgl);
> >  			vecs[i].len = min(sg_dma_len(sgl), len_total);
> >  			len_total -= vecs[i].len;
> > @@ -133,6 +139,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> >  		 * before it can run, so always set the EOT flag.
> >  		 */
> >  		flags |= DMA_PREP_LOAD_EOT;
> > +		nents = i;
> > +
> >  		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
> >  							 vecs, nents, dma_dir,
> >  							 flags);
> > 
> > ---
> > base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
> > change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
> > --
> > 
> > Thanks!
> > - Nuno Sá
> > 
>