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

Keguang Zhang <[email protected]> Fri, 31 Jul 2026 19:25:48 +0800
Newsgroups org.kernel.vger.dmaengine,org.kernel.vger.linux-kernel,org.kernel.vger.linux-mips
Message-ID <CAJhJPsUmpie6NkrzuTK-8C4NP2FFKQMG3nY8626_C9Qjdu4M0g@mail.gmail.com>
Reviewed-by: Keguang Zhang <[email protected]>
Tested-by: Keguang Zhang <[email protected]> # on LS1B & LS1C

On Wed, Jul 29, 2026 at 10:32=E2=80=AFPM Mahad Ibrahim
<[email protected]> 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.
>
> Signed-off-by: Mahad Ibrahim <[email protected]>
> ---
>
> 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/loong=
son/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 dm=
a_chan *dchan,
>
>                         /* locate the current lli */
>                         next_phys =3D chan->curr_lli->hw[LS1X_DMADESC_NEX=
T];
> -                       list_for_each_entry(lli, &desc->lli_list, node)
> -                               if (lli->hw[LS1X_DMADESC_NEXT] =3D=3D nex=
t_phys)
> -                                       break;
> +                       list_for_each_entry(lli, &desc->lli_list, node) {
> +                               if (lli->hw[LS1X_DMADESC_NEXT] !=3D next_=
phys)
> +                                       continue;
>
> -                       dev_dbg(chan2dev(dchan), "current lli_phys=3D%pad=
",
> -                               &lli->phys);
> +                               dev_dbg(chan2dev(dchan), "current lli_phy=
s=3D%pad\n",
> +                                       &lli->phys);
>
> -                       /* count the residues */
> -                       list_for_each_entry_from(lli, &desc->lli_list, no=
de)
> -                               bytes +=3D lli->hw[LS1X_DMADESC_LENGTH] *
> -                                        chan->bus_width;
> +                               /* count the residues */
> +                               list_for_each_entry_from(lli, &desc->lli_=
list, node)
> +                                       bytes +=3D lli->hw[LS1X_DMADESC_L=
ENGTH] *
> +                                                chan->bus_width;
> +
> +                               dma_set_residue(state, bytes);
> +                               return status;
> +                       }
> +
> +                       dev_warn(chan2dev(dchan),
> +                                "unable to locate current lli.\n");
>                 }
>         }
>
> -       dma_set_residue(state, bytes);
> -
>         return status;
>  }
>
> --
> 2.54.0
>


--=20
Best regards,

Keguang Zhang