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

[Keguang Zhang]

Hi Mahad,

Thanks for catching this use-after-iterator issue.

On Mon, Jul 20, 2026 at 10:25 PM Mahad Ibrahim
<mahad.ibrahim.dev@xxxxxxxxx> 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
> 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@xxxxxxxxx>
> ---
>  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);

However, I don't think falling back to list_first_entry() when no
match is found is appropriate. Since next_phys should always match one
of the LLIs in the descriptor chain, failing to find a match should be
treated as an unexpected condition rather than silently falling back
to the first LLI.
I would suggest something like:

       scoped_guard(spinlock_irqsave, &chan->vc.lock) {
               vd = vchan_find_desc(&chan->vc, cookie);
               if (vd) {
                       struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vd);
                       struct ls1x_dma_lli *lli;
                       dma_addr_t next_phys;

                       /* get the current lli */
                       if (ls1x_dma_query(chan, &chan->curr_lli->phys))
                               return status;

                       /* 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)
                                       continue;

                               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;

                               dma_set_residue(state, bytes);
                               return status;
                       }

                       dev_warn(chan2dev(dchan), "unable to locate
current LLI\n");
               }
       }

       return status;

> +                       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
>


-- 
Best regards,

Keguang Zhang