[PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors

[PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors

[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
internally expands to the container_of macro to find the current
ls1x_dma_lli's next-descriptor pointer to compare against. However a
problem occurs when the loop reaches the final lli list element and wraps
around to the head. The head is a different struct ls1x_dma_desc. It goes
down to the offset the node member would be at in ls1x_dma_lli inside of
ls1x_dma_desc.

This leaves the lli pointer somewhere inside of ls1x_dma_desc at the end
of the loop. The subsequent phys debug value read therefore would print
garbage as a valid phys address.

The primary bug occurs in the residue calculation list_for_each_entry_from
macro. The lli pointer still points to somewhere inside of ls1x_dma_desc,
when the list_for_each_entry_from performs the same wrong offset
calculation on ls1x_dma_desc to determine the stop condition of the loop,
it lands at the same offset that lli pointer is on, and the loop
immediately stops.

This produces a 0 residue value, while it should be the entire length of
the lli list chain. The driver advertises DMA_RESIDUE_GRANULARITY_SEGMENT,
so a 0 residue would mean the transfer has completed.

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

Note the check still reports this line after the fix; the remaining use
is safe (the cursor is only dereferenced on the match path, or reset to
the first entry otherwise).

I did not see a bug upstream detailing this error, nor do I have the
hardware to confirm this bug or error, all this is from a pure code
examination.

As I do not possess the hardware, I cannot test the patch. Compile tested
only with mips64-linux-gnu-gcc.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 drivers/dma/loongson/loongson1-apb-dma.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
index 89786cbd20ab..8bb6e6e5719d 100644
--- a/drivers/dma/loongson/loongson1-apb-dma.c
+++ b/drivers/dma/loongson/loongson1-apb-dma.c
@@ -439,6 +439,7 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
 			struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vd);
 			struct ls1x_dma_lli *lli;
 			dma_addr_t next_phys;
+			bool found = false;
 
 			/* get the current lli */
 			if (ls1x_dma_query(chan, &chan->curr_lli->phys))
@@ -447,11 +448,17 @@ 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)
+				if (lli->hw[LS1X_DMADESC_NEXT] == next_phys) {
+					found = true;
 					break;
-
-			dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
-				&lli->phys);
+				}
+
+			if (!found)
+				lli = list_first_entry(&desc->lli_list,
+						       struct ls1x_dma_lli, node);
+			else
+				dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
+					&lli->phys);
 
 			/* count the residues */
 			list_for_each_entry_from(lli, &desc->lli_list, node)
-- 
2.54.0