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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Out-of-bounds array access in `axi_chan_block_xfer_complete()` due to using a channel-wide allocation count instead of a descriptor-specific count.
- [Critical] Unbalanced error cleanup leads to fatal NULL pointer dereference in `axi_desc_put()`.
--

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

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

This patch addresses an out-of-bounds array read in axi_chan_list_dump_lli().
It modifies the loop to use the descriptor-local count instead of the
channel-wide allocation count. This prevents reading past the end of the array
when multiple transactions are present.

> 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 isn't a bug introduced by this patch, but the exact same logic flaw
appears to exist 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);
    [ ... ]
	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);
    [ ... ]
}

If multiple transfers are queued or prepared, can the channel-wide count
exceed the transaction-local desc->nr_hw_descs count and cause an
out-of-bounds access of the desc->hw_desc[] array here as well?

Could this out-of-bounds memory be dereferenced as an LLI descriptor pointer
and subsequently cause memory corruption when written to?

[Severity: Critical]
This isn't a bug introduced by this patch, but is there also a regression
with unbalanced error cleanup causing a NULL pointer dereference?

In dw_axi_dma_chan_prep_cyclic(), if an error occurs midway through setting up
the segments:

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);
		if (status < 0)
			goto err_desc_get;
    [ ... ]
}

The error path will call axi_desc_put(), which loops over the entire
desc->nr_hw_descs array unconditionally:

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

Since desc->hw_desc is zero-initialized and trailing elements may not have
been populated yet, won't hw_desc->lli be NULL for those elements?

Since dma_pool_free() does not handle NULL virtual addresses, will this
lead to a panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-dma-fix-v3-0-414c6449fbcc@ultrarisc.com?part=2