From: Frank Li <Frank.li@oss.nxp.com>
To: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
Cc: Keguang Zhang <keguang.zhang@gmail.com>,
Vinod Koul <vkoul@kernel.org>, Frank Li <Frank.Li@kernel.org>,
linux-mips@vger.kernel.org, dmaengine@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] dmaengine: loongson1-apb-dma: avoid using iterator variable after list_for_each_entry()
Date: Thu, 17 Sep 2026 12:03:50 -0500 [thread overview]
Message-ID: <aqwddneAK_yPAkVS@SMW015318> (raw)
In-Reply-To: <20260917163025.7171-1-mahad.ibrahim.dev@gmail.com>
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
>
prev parent reply other threads:[~2026-09-17 17:04 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-17 16:30 Mahad Ibrahim
2026-09-17 17:03 ` Frank Li [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aqwddneAK_yPAkVS@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Frank.Li@kernel.org \
--cc=dmaengine@vger.kernel.org \
--cc=keguang.zhang@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=mahad.ibrahim.dev@gmail.com \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®