All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jia Wang" <wangjia@ultrarisc.com>
Cc: dmaengine@vger.kernel.org, Frank.Li@kernel.org, vkoul@kernel.org
Subject: Re: [PATCH v2 2/4] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access
Date: Tue, 01 Sep 2026 06:29:42 +0000	[thread overview]
Message-ID: <20260901062942.DF2431F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260901-dma-fix-v2-2-d7f0459ebb14@ultrarisc.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The patch fixes an out-of-bounds access in `axi_chan_list_dump_lli()` by using the transaction-local `desc->nr_hw_descs` instead of the channel-wide `chan->descs_allocated`. However, it completely misses the exact same bug in `axi_chan_block_xfer_complete()` which can lead to memory corruption.
- [Critical] Flawed calculations for `segment_len` and loop bounds in both `dw_axi_dma_chan_prep_slave_sg` and `dw_axi_dma_chan_prep_cyclic` lead to data truncation, DMA overruns, and NULL pointer dereferences when buffer lengths are not perfectly divisible by the calculated `segment_len`.
--

commit 5ff010f18efeeec7fde7f85a181da39448ad009d
Author: Jia Wang <wangjia@ultrarisc.com>

dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access

This commit fixes an out-of-bounds access in axi_chan_list_dump_lli() by
using the transaction-local descriptor count instead of the channel-wide
count.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index 742e08cfab435..61230d2b1c561 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -1052,10 +1052,7 @@ static void axi_chan_dump_lli(struct axi_dma_chan *chan,
>  static void axi_chan_list_dump_lli(struct axi_dma_chan *chan,
>  				   struct axi_dma_desc *desc_head)
>  {
> -	int count = atomic_read(&chan->descs_allocated);
> -	int i;
> -
> -	for (i = 0; i < count; i++)
> +	for (unsigned int i = 0; i < desc_head->nr_hw_descs; i++)
>  		axi_chan_dump_lli(chan, &desc_head->hw_desc[i]);
>  }

[Severity: Critical]
This is a pre-existing issue, but does axi_chan_block_xfer_complete() have
the same out-of-bounds access regression?

If multiple transactions are prepared, chan->descs_allocated can exceed the
transaction-local desc->nr_hw_descs:

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_chan_block_xfer_complete() {
    ...
    int count = atomic_read(&chan->descs_allocated);
    ...
    if (chan->cyclic) {
        desc = vd_to_axi_desc(vd);
        if (desc) {
            llp = lo_hi_readq(chan->chan_regs + CH_LLP);
            for (i = 0; i < count; i++) {
                hw_desc = &desc->hw_desc[i];
                if (hw_desc->llp == llp) {
                    axi_chan_irq_clear(chan, hw_desc->lli->status_lo);
                    hw_desc->lli->ctl_hi |= cpu_to_le32(CH_CTL_H_LLI_VALID);
    ...
}

Could this cause an out-of-bounds read on the desc->hw_desc array, and
corrupt memory when writing to hw_desc->lli->ctl_hi?

[Severity: Critical]
This is also a pre-existing issue, but can the segment loop bounds cause
data truncation or memory overruns?

In dw_axi_dma_chan_prep_slave_sg(), if len is not perfectly divisible by
segment_len, the loop terminates prematurely:

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_slave_sg() {
    ...
    do {
        hw_desc = &desc->hw_desc[loop++];
        status = dw_axi_dma_set_hw_desc(chan, hw_desc, mem, segment_len);
        ...
        len -= segment_len;
        mem += segment_len;
    } while (len >= segment_len);
    ...
}

Does this drop the remainder bytes and leave trailing descriptors
uninitialized, causing a NULL dereference when set_desc_last() is called?

Similarly, in dw_axi_dma_chan_prep_cyclic():

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_cyclic() {
    ...
    for (i = 0; i < total_segments; i++) {
        hw_desc = &desc->hw_desc[i];
        status = dw_axi_dma_set_hw_desc(chan, hw_desc, src_addr, segment_len);
        ...
        src_addr += segment_len;
    }
    ...
}

Can this unconditionally program the DMA to transfer segment_len for each
segment, exceeding the requested period_len and reading or writing past the
end of the provided DMA buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-dma-fix-v2-0-d7f0459ebb14@ultrarisc.com?part=2

  reply	other threads:[~2026-09-01  6:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01  6:16 [PATCH v2 0/4] dmaengine: dw-axi-dmac: Fix burst encoding, LLI dump and priority Jia Wang
2026-09-01  6:16 ` [PATCH v2 1/4] dmaengine: dw-axi-dmac: Fix AXI burst length encoding Jia Wang
2026-09-01  6:29   ` sashiko-bot
2026-09-01  6:16 ` [PATCH v2 2/4] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access Jia Wang
2026-09-01  6:29   ` sashiko-bot [this message]
2026-09-01 18:53   ` Frank Li
2026-09-01  6:16 ` [PATCH v2 3/4] dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position Jia Wang
2026-09-01  6:28   ` sashiko-bot
2026-09-01 18:55   ` Frank Li
2026-09-01  6:16 ` [PATCH v2 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers Jia Wang
2026-09-01  7:02   ` Andy Shevchenko
2026-09-01  8:49     ` Jia Wang

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=20260901062942.DF2431F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    --cc=wangjia@ultrarisc.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.