mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] dmaengine: loongson1-apb-dma: avoid using iterator variable after list_for_each_entry()
@ 2026-09-17 16:30 Mahad Ibrahim
  2026-09-17 17:03 ` Frank Li
  0 siblings, 1 reply; 2+ messages in thread
From: Mahad Ibrahim @ 2026-09-17 16:30 UTC (permalink / raw)
  To: Keguang Zhang, Vinod Koul
  Cc: Frank Li, Frank Li, linux-mips, dmaengine, linux-kernel, Mahad Ibrahim

ls1x_dma_tx_status() locates the descriptor actively being processed by
walking the LLI list and comparing the hardware reported next descriptor
pointer against each element's next-descriptor pointer.

A list_for_each_entry macro is used in the comparison phase, which at
the end of the loop leaves the lli pointer at the currently executing LLI.
However this also subsequently runs for a non-match lli, in which it
points at the head. This causes a type confusion bug which treats the
head, which is a ls1x_dma_desc, as a ls1x_dma_lli object. Additionally it
goes forwards and prints garbage via the dev_dbg.

Fix the type confusion bug by only allowing matched LLI descriptor chains
to print the current LLI and residue calculation, as failing to match
should be treated as an unexpected condition.

Found by the following Coccinelle check:

  scripts/coccinelle/iterators/use_after_iter.cocci

  drivers/dma/loongson/loongson1-apb-dma.c:461:6-9: ERROR: invalid
  reference to the index variable of the iterator on line 450

Compile test only; Hardware testing by Keguang Zhang.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
Reviewed-by: Keguang Zhang <keguang.zhang@gmail.com>
Tested-by: Keguang Zhang <keguang.zhang@gmail.com> # on LS1B & LS1C
---

v2:
- encapsulate residue calculation and dev_dbg inside the
  list_for_each_entry() macro. Treat non-matching LLI as an unexpected
  case.
- link: https://lore.kernel.org/all/20260729143247.6111-1-mahad.ibrahim.dev@gmail.com/


v3:
- Collect tags from Keguang Zhang.
- Repost patch in a new thread.
- Change chan2dev to the newer dmaengine_chan_dev.


 drivers/dma/loongson/loongson1-apb-dma.c | 27 ++++++++++++++----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
index 46b4bfef45e2..e62ab26c8327 100644
--- a/drivers/dma/loongson/loongson1-apb-dma.c
+++ b/drivers/dma/loongson/loongson1-apb-dma.c
@@ -441,22 +441,27 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
 
 			/* locate the current lli */
 			next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
-			list_for_each_entry(lli, &desc->lli_list, node)
-				if (lli->hw[LS1X_DMADESC_NEXT] == next_phys)
-					break;
+			list_for_each_entry(lli, &desc->lli_list, node) {
+				if (lli->hw[LS1X_DMADESC_NEXT] != next_phys)
+					continue;
 
-			dev_dbg(dmaengine_chan_dev(dchan), "current lli_phys=%pad",
-				&lli->phys);
+				dev_dbg(dmaengine_chan_dev(dchan), "current lli_phys=%pad\n",
+					&lli->phys);
 
-			/* count the residues */
-			list_for_each_entry_from(lli, &desc->lli_list, node)
-				bytes += lli->hw[LS1X_DMADESC_LENGTH] *
-					 chan->bus_width;
+				/* count the residues */
+				list_for_each_entry_from(lli, &desc->lli_list, node)
+					bytes += lli->hw[LS1X_DMADESC_LENGTH] *
+						 chan->bus_width;
+
+				dma_set_residue(state, bytes);
+				return status;
+			}
+
+			dev_warn(dmaengine_chan_dev(dchan),
+				 "unable to locate current lli.\n");
 		}
 	}
 
-	dma_set_residue(state, bytes);
-
 	return status;
 }
 
-- 
2.54.0


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

* Re: [PATCH v3] dmaengine: loongson1-apb-dma: avoid using iterator variable after list_for_each_entry()
  2026-09-17 16:30 [PATCH v3] dmaengine: loongson1-apb-dma: avoid using iterator variable after list_for_each_entry() Mahad Ibrahim
@ 2026-09-17 17:03 ` Frank Li
  0 siblings, 0 replies; 2+ messages in thread
From: Frank Li @ 2026-09-17 17:03 UTC (permalink / raw)
  To: Mahad Ibrahim
  Cc: Keguang Zhang, Vinod Koul, Frank Li, linux-mips, dmaengine, linux-kernel

On Thu, Sep 17, 2026 at 04:30:25PM +0000, Mahad Ibrahim wrote:
> ls1x_dma_tx_status() locates the descriptor actively being processed by
> walking the LLI list and comparing the hardware reported next descriptor
> pointer against each element's next-descriptor pointer.
>
> A list_for_each_entry macro is used in the comparison phase, which at
> the end of the loop leaves the lli pointer at the currently executing LLI.
> However this also subsequently runs for a non-match lli, in which it
> points at the head. This causes a type confusion bug which treats the
> head, which is a ls1x_dma_desc, as a ls1x_dma_lli object. Additionally it
> goes forwards and prints garbage via the dev_dbg.
>
> Fix the type confusion bug by only allowing matched LLI descriptor chains
> to print the current LLI and residue calculation, as failing to match
> should be treated as an unexpected condition.
>
> Found by the following Coccinelle check:
>
>   scripts/coccinelle/iterators/use_after_iter.cocci
>
>   drivers/dma/loongson/loongson1-apb-dma.c:461:6-9: ERROR: invalid
>   reference to the index variable of the iterator on line 450
>
> Compile test only; Hardware testing by Keguang Zhang.
>
> Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
> Reviewed-by: Keguang Zhang <keguang.zhang@gmail.com>
> Tested-by: Keguang Zhang <keguang.zhang@gmail.com> # on LS1B & LS1C
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> v2:
> - encapsulate residue calculation and dev_dbg inside the
>   list_for_each_entry() macro. Treat non-matching LLI as an unexpected
>   case.
> - link: https://lore.kernel.org/all/20260729143247.6111-1-mahad.ibrahim.dev@gmail.com/
>
>
> v3:
> - Collect tags from Keguang Zhang.
> - Repost patch in a new thread.
> - Change chan2dev to the newer dmaengine_chan_dev.
>
>
>  drivers/dma/loongson/loongson1-apb-dma.c | 27 ++++++++++++++----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
> index 46b4bfef45e2..e62ab26c8327 100644
> --- a/drivers/dma/loongson/loongson1-apb-dma.c
> +++ b/drivers/dma/loongson/loongson1-apb-dma.c
> @@ -441,22 +441,27 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
>
>  			/* locate the current lli */
>  			next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
> -			list_for_each_entry(lli, &desc->lli_list, node)
> -				if (lli->hw[LS1X_DMADESC_NEXT] == next_phys)
> -					break;
> +			list_for_each_entry(lli, &desc->lli_list, node) {
> +				if (lli->hw[LS1X_DMADESC_NEXT] != next_phys)
> +					continue;
>
> -			dev_dbg(dmaengine_chan_dev(dchan), "current lli_phys=%pad",
> -				&lli->phys);
> +				dev_dbg(dmaengine_chan_dev(dchan), "current lli_phys=%pad\n",
> +					&lli->phys);
>
> -			/* count the residues */
> -			list_for_each_entry_from(lli, &desc->lli_list, node)
> -				bytes += lli->hw[LS1X_DMADESC_LENGTH] *
> -					 chan->bus_width;
> +				/* count the residues */
> +				list_for_each_entry_from(lli, &desc->lli_list, node)
> +					bytes += lli->hw[LS1X_DMADESC_LENGTH] *
> +						 chan->bus_width;
> +
> +				dma_set_residue(state, bytes);
> +				return status;
> +			}
> +
> +			dev_warn(dmaengine_chan_dev(dchan),
> +				 "unable to locate current lli.\n");
>  		}
>  	}
>
> -	dma_set_residue(state, bytes);
> -
>  	return status;
>  }
>
> --
> 2.54.0
>

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

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

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-17 16:30 [PATCH v3] dmaengine: loongson1-apb-dma: avoid using iterator variable after list_for_each_entry() Mahad Ibrahim
2026-09-17 17:03 ` Frank Li

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®