Re: [PATCH v2] dmaengine: loongson1-apb-dma: avoid using iterator variable after list_for_each_entry()

[Binbin Zhou]

Hi Frank:

Sorry for the late reply.

On 2026/8/15 03:33, Frank Li wrote:
> On Wed, Jul 29, 2026 at 02:32:47PM +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
> >
> > 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.
>
> Avoid post new patch on old email thread.
>
> Just simple said
>
> Compile test only.
>
> > Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@xxxxxxxxx>
> > ---
> >
> > v2:
> > - encapsulate residue calculation and dev_dbg inside the
> >    list_for_each_entry() macro. Treat non-matching LLI as an unexpected
> >    case.
> >
> >
> >   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 89786cbd20ab..8658d5377795 100644
> > --- a/drivers/dma/loongson/loongson1-apb-dma.c
> > +++ b/drivers/dma/loongson/loongson1-apb-dma.c
> > @@ -446,22 +446,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(chan2dev(dchan), "current lli_phys=%pad",
> > -				&lli->phys);
> > +				dev_dbg(chan2dev(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(chan2dev(dchan),
> > +				 "unable to locate current lli.\n");
>
> most likely logic is correct, it is not obvious, hope some one who have
> hardware help test it.
>
> Binbin Zhou just add add this driver recently, he should have hardware to
> test it.

Emm, sorry, I don't have any test equipment available right now. As far 
as I know, Keguang is the maintainer of this driver, and he has the 
necessary test equipment.
> Frank
> >   		}
> >   	}
> >
> > -	dma_set_residue(state, bytes);
> > -
> >   	return status;
> >   }
> >
> > --
> > 2.54.0

--
Thanks.
Binbin