Re: [PATCH 2/2] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The exact same out-of-bounds array access bug that this commit fixes in `axi_chan_list_dump_lli()` remains unpatched in `axi_chan_block_xfer_complete()`, compounded by a TOCTOU race.
- [High] A flawed SG iteration loop in `dw_axi_dma_chan_prep_slave_sg()` exits prematurely, silently truncating transfers and causing an immediate NULL pointer dereference panic.
- [High] On descriptor allocation error paths, `axi_desc_put()` will attempt to free `NULL` pointers to the DMA pool and incorrectly subtract unallocated counts from `chan->descs_allocated`, leading to kernel panics and counter corruption.
--

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

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

axi_chan_list_dump_lli() uses the channel-wide descs_allocated count to
walk the hw_desc[] array of a single transaction. If multiple
transactions have allocated LLIs, the channel count can exceed the
transaction-local nr_hw_descs and make the DMA error path read past the
end of hw_desc[].

> 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..858004b5a8467 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,9 @@ 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 (i = 0; i < desc_head->nr_hw_descs; i++)
>  		axi_chan_dump_lli(chan, &desc_head->hw_desc[i]);
>  }

[Severity: High]
This is a pre-existing issue, but the exact same out-of-bounds array access
bug that this commit fixes in axi_chan_list_dump_lli() remains unpatched in
axi_chan_block_xfer_complete():

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_chan_block_xfer_complete() {
    ...
    int count = atomic_read(&chan->descs_allocated);
    ...
    for (i = 0; i < count; i++) {
        hw_desc = &desc->hw_desc[i];
    ...
}

Does this function also need to be updated to use desc->nr_hw_descs for
its loop limit? Using the channel-wide count (which is fetched outside the
channel spinlock, presenting a TOCTOU race) could cause an out-of-bounds read
on desc->hw_desc[]. If the garbage memory happens to match llp, it will modify
the memory via hw_desc->lli->ctl_hi.


[Severity: High]
This is a pre-existing issue, but looking at dw_axi_dma_chan_prep_slave_sg(),
is there a flaw in the SG iteration loop?

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_slave_sg() {
    ...
    segment_len = DIV_ROUND_UP(sg_dma_len(sg), num_segments);
    ...
    } while (len >= segment_len);
    ...
    write_desc_llp(hw_desc, llp | lms);
    ...
}

If a DMA client submits a scatter-gather list where the length does not evenly
divide into the hardware maximum block size (e.g., length 4097 and block
length 4096 gives a segment_len of 2049, leaving 2048 bytes), the condition
while (len >= segment_len) evaluates to false and exits the loop prematurely.

Does this silent truncation leave trailing descriptors uninitialized, causing an
immediate NULL pointer dereference panic when write_desc_llp() tries to access
hw_desc->lli?


[Severity: High]
This is a pre-existing issue, but does axi_desc_put() safely handle error
rollbacks?

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_desc_put() {
    ...
    for (i = 0; i < desc->nr_hw_descs; i++) {
        ...
        dma_pool_free(chan->desc_pool, hw_desc->lli, hw_desc->llp);
        ...
    }
    ...
    atomic_sub(descs_put, &chan->descs_allocated);
}

If descriptor initialization fails midway (e.g., due to OOM causing -ENOMEM),
the error path calls axi_desc_put(). Because it unconditionally iterates over
all desc->nr_hw_descs elements, won't it pass NULL hw_desc->lli pointers to
dma_pool_free() for unallocated entries, resulting in a panic?

Additionally, will subtracting the full capacity (descs_put) from
chan->descs_allocated cause an underflow when only partially allocated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-dma-fix-v1-0-a6947f487e07@ultrarisc.com?part=2