Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Tasklets created for DMA channels are not killed during driver removal, risking use-after-free panics if scheduled after devm allocations are freed.
--
commit fe57b7262d586b3f961d1d34cf7a9a0acf70d3c7
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: use dma_chan_name() helper to get per-channel device name
This commit updates various DMA engine drivers to use the existing
dma_chan_name() helper instead of open-coding the per-channel device name
retrieval. The change simplifies the code and improves consistency across
the subsystem without altering functionality.
[ ... ]
> diff --git a/drivers/dma/loongson/loongson2-apb-dma.c b/drivers/dma/loongson/loongson2-apb-dma.c
> index aceb069e71fca..7c3d7c3273955 100644
> --- a/drivers/dma/loongson/loongson2-apb-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-dma.c
> @@ -282,7 +282,7 @@ static int ls2x_dma_alloc_chan_resources(struct dma_chan *chan)
> struct ls2x_dma_chan *lchan = to_ldma_chan(chan);
>
> /* Create a pool of consistent memory blocks for hardware descriptors */
> - lchan->pool = dma_pool_create(dev_name(chan2dev(chan)),
> + lchan->pool = dma_pool_create(dma_chan_name(chan),
> chan->device->dev, PAGE_SIZE,
> __alignof__(struct ls2x_dma_hw_desc), 0);
> if (!lchan->pool) {
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing
issue in ls2x_dma_remove() missing tasklet cleanup.
Should ls2x_dma_remove() iterate over the channels and call
tasklet_kill(&chan->vchan.task) before returning?
The driver relies on devm allocations for channel memory, which are
automatically freed immediately after remove() completes. If the DMA driver
is unbound shortly after an interrupt fires, the interrupt handler schedules
the channel's tasklet. If this scheduled tasklet executes after the memory
is freed, can it dereference invalid memory and cause a use-after-free
panic?
[ ... ]
> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> index 386e85cd4882a..6b29e15b0616c 100644
> --- a/drivers/dma/mmp_pdma.c
> +++ b/drivers/dma/mmp_pdma.c
> @@ -565,7 +565,7 @@ static int mmp_pdma_alloc_chan_resources(struct dma_chan *dchan)
> if (chan->desc_pool)
> return 1;
>
> - chan->desc_pool = dma_pool_create(dev_name(&dchan->dev->device),
> + chan->desc_pool = dma_pool_create(dma_chan_name(dchan),
> chan->dev,
> sizeof(struct mmp_pdma_desc_sw),
> __alignof__(struct mmp_pdma_desc_sw),
[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a similar
pre-existing issue in mmp_pdma_remove().
Does mmp_pdma_remove() also need a loop over the channels to call
tasklet_kill(&chan->tasklet)?
Like the Loongson driver, this driver also uses devm allocations for
channel structures and could experience a use-after-free panic if a tasklet
runs after mmp_pdma_remove() finishes and the memory is freed.
@@ -760,7 +760,7 @@ static int jz4780_dma_alloc_chan_resources(struct dma_chan *chan)
{
struct jz4780_dma_chan *jzchan = to_jz4780_dma_chan(chan);
- jzchan->desc_pool = dma_pool_create(dev_name(&chan->dev->device),
+ jzchan->desc_pool = dma_pool_create(dma_chan_name(chan),
chan->device->dev,
JZ_DMA_DESC_BLOCK_SIZE,
PAGE_SIZE, 0);
@@ -531,7 +531,7 @@ static int dma_chan_alloc_chan_resources(struct dma_chan *dchan)
}
/* LLI address must be aligned to a 64-byte boundary */
- chan->desc_pool = dma_pool_create(dev_name(chan2dev(chan)),
+ chan->desc_pool = dma_pool_create(dma_chan_name(dchan),
chan->chip->dev,
sizeof(struct axi_dma_lli),
64, 0);
@@ -507,7 +507,7 @@ static int idma64_alloc_chan_resources(struct dma_chan *chan)
struct idma64_chan *idma64c = to_idma64_chan(chan);
/* Create a pool of consistent memory blocks for hardware descriptors */
- idma64c->pool = dma_pool_create(dev_name(chan2dev(chan)),
+ idma64c->pool = dma_pool_create(dma_chan_name(chan),
chan->device->dev,
sizeof(struct idma64_lli), 8, 0);
if (!idma64c->pool) {
@@ -282,7 +282,7 @@ static int ls2x_dma_alloc_chan_resources(struct dma_chan *chan)
struct ls2x_dma_chan *lchan = to_ldma_chan(chan);
/* Create a pool of consistent memory blocks for hardware descriptors */
- lchan->pool = dma_pool_create(dev_name(chan2dev(chan)),
+ lchan->pool = dma_pool_create(dma_chan_name(chan),
chan->device->dev, PAGE_SIZE,
__alignof__(struct ls2x_dma_hw_desc), 0);
if (!lchan->pool) {
@@ -565,7 +565,7 @@ static int mmp_pdma_alloc_chan_resources(struct dma_chan *dchan)
if (chan->desc_pool)
return 1;
- chan->desc_pool = dma_pool_create(dev_name(&dchan->dev->device),
+ chan->desc_pool = dma_pool_create(dma_chan_name(dchan),
chan->dev,
sizeof(struct mmp_pdma_desc_sw),
__alignof__(struct mmp_pdma_desc_sw),
@@ -268,7 +268,7 @@ static int st_fdma_alloc_chan_res(struct dma_chan *chan)
struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
/* Create the dma pool for descriptor allocation */
- fchan->node_pool = dma_pool_create(dev_name(&chan->dev->device),
+ fchan->node_pool = dma_pool_create(dma_chan_name(chan),
fchan->fdev->dev,
sizeof(struct st_fdma_hw_node),
__alignof__(struct st_fdma_hw_node),
@@ -1126,7 +1126,7 @@ static int stm32_dma3_alloc_chan_resources(struct dma_chan *c)
goto err_put_sync;
}
- chan->lli_pool = dmam_pool_create(dev_name(&c->dev->device), c->device->dev,
+ chan->lli_pool = dmam_pool_create(dma_chan_name(c), c->device->dev,
sizeof(struct stm32_dma3_hwdesc),
__alignof__(struct stm32_dma3_hwdesc), SZ_64K);
if (!chan->lli_pool) {
@@ -1484,7 +1484,7 @@ static int stm32_mdma_alloc_chan_resources(struct dma_chan *c)
struct stm32_mdma_device *dmadev = stm32_mdma_get_dev(chan);
int ret;
- chan->desc_pool = dmam_pool_create(dev_name(&c->dev->device),
+ chan->desc_pool = dmam_pool_create(dma_chan_name(c),
c->device->dev,
sizeof(struct stm32_mdma_hwdesc),
__alignof__(struct stm32_mdma_hwdesc),
@@ -11,7 +11,7 @@ TRACE_EVENT(tegra_dma_tx_status,
TP_PROTO(struct dma_chan *dc, dma_cookie_t cookie, struct dma_tx_state *state),
TP_ARGS(dc, cookie, state),
TP_STRUCT__entry(
- __string(chan, dev_name(&dc->dev->device))
+ __string(chan, dma_chan_name(dc))
__field(dma_cookie_t, cookie)
__field(__u32, residue)
),
@@ -28,7 +28,7 @@ TRACE_EVENT(tegra_dma_complete_cb,
TP_PROTO(struct dma_chan *dc, int count, void *ptr),
TP_ARGS(dc, count, ptr),
TP_STRUCT__entry(
- __string(chan, dev_name(&dc->dev->device))
+ __string(chan, dma_chan_name(dc))
__field(int, count)
__field(void *, ptr)
),
@@ -45,7 +45,7 @@ TRACE_EVENT(tegra_dma_isr,
TP_PROTO(struct dma_chan *dc, int irq),
TP_ARGS(dc, irq),
TP_STRUCT__entry(
- __string(chan, dev_name(&dc->dev->device))
+ __string(chan, dma_chan_name(dc))
__field(int, irq)
),
TP_fast_assign(