Re: [PATCH v3 3/4] dmaengine: add (dmaengine|vchan)_chan_dev() helper

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Descriptor preparation functions return ERR_PTR instead of NULL and leak memory on failure.
- [High] Out-of-bounds memory access in at_xdmac_tx_status() when the transfers list is empty.
--

commit 3b17906d45663cb4c1aae18a74a2d9d31cfc3f4d
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: add (dmaengine|vchan)_chan_dev() helper
    
This commit adds new helper functions (dmaengine|vchan)_chan_dev() to
access the DMA channel device. It replaces private chan2dev() implementations
in individual DMA engine drivers to provide a cleaner common API.

> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index 901971e8bae69..d40a8b40dddc3 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c

[ ... ]

> @@ -1667,7 +1662,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>  
>  	dma_set_residue(txstate, residue);
>  
> -	dev_dbg(chan2dev(chan),
> +	dev_dbg(dmaengine_chan_dev(chan),
>  		 "%s: desc=0x%p, tx_dma_desc.phys=%pad, tx_status=%d, cookie=%d, residue=%d\n",
>  		 __func__, desc, &desc->tx_dma_desc.phys, ret, cookie, residue);

[Severity: High]
This is a pre-existing issue, but is there a potential out-of-bounds access
in at_xdmac_tx_status() when the transfers list is empty?

If dmaengine_tx_status() is called after at_xdmac_device_terminate_all()
empties the atchan->xfers_list, dma_cookie_status() might still return
DMA_IN_PROGRESS because the cookie is not marked as completed.

Earlier in at_xdmac_tx_status(), the code blindly calls list_first_entry()
on atchan->xfers_list without checking if it is empty, and then accesses
desc->active_xfer. Since list_first_entry() does not verify emptiness,
this interprets the list_head pointer as a structure, which could lead
to an invalid memory access or a crash.

[ ... ]

> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index 2f1c8c4fd0654..755363f4bfa2c 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c

[ ... ]

> @@ -409,7 +404,7 @@ loongson2_cmc_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  
>  		num_items = DIV_ROUND_UP(sg_dma_len(sg), buswidth);
>  		if (num_items >= LOONSON2_CMCDMA_MAX_DATA_ITEMS) {
> -			dev_err(chan2dev(lchan), "Number of items not supported\n");
> +			dev_err(vchan_chan_dev(&lchan->vchan), "Number of items not supported\n");
>  			kfree(desc);
>  			return ERR_PTR(-EINVAL);
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but shouldn't the dmaengine API
functions device_prep_slave_sg() and device_prep_dma_cyclic() return NULL
on failure rather than ERR_PTR()?

In loongson2_cmc_dma_prep_slave_sg(), if loongson2_cmc_dma_set_xfer_param()
fails or the item count is unsupported, it returns ERR_PTR. Since callers
typically check for !desc on failure, returning ERR_PTR means the error
check passes, which could lead to a kernel panic when the invalid pointer
is dereferenced.

Additionally, earlier in the same loop, if set_xfer_param() fails, the
previously allocated desc via kzalloc_flex() appears to be leaked before
returning ERR_PTR(ret).

> @@ -447,7 +442,7 @@ loongson2_cmc_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, si
>  
>  	num_items = DIV_ROUND_UP(period_len, buswidth);
>  	if (num_items >= LOONSON2_CMCDMA_MAX_DATA_ITEMS) {
> -		dev_err(chan2dev(lchan), "Number of items not supported\n");
> +		dev_err(vchan_chan_dev(&lchan->vchan), "Number of items not supported\n");
>  		return ERR_PTR(-EINVAL);
>  	}

[Severity: High]
This is also a pre-existing issue, but similar to the above, does this
return value violate the API expectations?

Returning ERR_PTR(-EINVAL) here and when loongson2_cmc_dma_set_xfer_param()
fails will likely bypass caller checks for NULL, potentially leading to a
crash on dereference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-dma-chan_dev-rename-v3-0-685601b0b788@nxp.com?part=3