Re: [PATCH v3 3/4] dmaengine: add (dmaengine|vchan)_chan_dev() helper
From: sashiko-bot@kernel.org
Date: 2026-09-04 22:02:53
Also in:
dmaengine, imx, linux-sunxi
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 [off-list ref]
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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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. [ ... ]
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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).
quoted hunk ↗ jump to hunk
@@ -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