Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Probe error path prematurely frees DMA channel structures and disables clocks before devm-managed IRQs are freed, leading to a Use-After-Free and potential bus hang.
--
--- Patch [1]: [PATCH 1/2] dmaengine: use dma_chan_name() helper to get per-channel device name ---
commit 77ba32b52a0d71c14db9e5276f81ba0fa2896131
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: use dma_chan_name() helper to get per-channel device name
This patch uses the existing dma_chan_name() helper to obtain the
per-channel device name instead of open-coding it. This simplifies the
code and improves consistency.
[ ... ]
> diff --git a/drivers/dma/stm32/stm32-dma.c b/drivers/dma/stm32/stm32-dma.c
> index d3ad78562a140..4e3ecb6763036 100644
> --- a/drivers/dma/stm32/stm32-dma.c
> +++ b/drivers/dma/stm32/stm32-dma.c
> @@ -1668,7 +1668,7 @@ static int stm32_dma_probe(struct platform_device *pdev)
>
> ret = devm_request_irq(&pdev->dev, chan->irq,
> stm32_dma_chan_irq, 0,
> - dev_name(chan2dev(chan)), chan);
> + dma_chan_name(&chan->vchan.chan), chan);
[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but doesn't the error path in stm32_dma_probe() in
drivers/dma/stm32/stm32-dma.c prematurely free DMA channel structures and
disable clocks before the devm-managed IRQs are freed?
In stm32_dma_probe(), IRQs are requested using devm_request_irq() and
passed dma_chan_name(&chan->vchan.chan) as the action name.
If a subsequent initialization step fails, the error path manually calls
dma_async_device_unregister(dd):
err_unregister:
dma_async_device_unregister(dd);
clk_free:
clk_disable_unprepare(dmadev->clk);
return ret;
This frees the channel device structures, including the embedded name
string, and clk_disable_unprepare(dmadev->clk) stops the hardware clock.
Because the IRQs are devres-managed, they remain active until the probe
function returns and the devres framework unwinds. During this window, if
/proc/interrupts is read, the kernel accesses the freed action->name
string, potentially triggering a Use-After-Free.
Furthermore, if a spurious interrupt fires during this window, the handler
stm32_dma_chan_irq() will read hardware registers with the device clock
disabled, which could trigger a Data Abort kernel panic.
> if (ret) {
> dev_err(&pdev->dev,
> "request_irq failed with err %d channel %d\n",
@@ -512,7 +512,7 @@ static int d350_alloc_chan_resources(struct dma_chan *chan)
{
struct d350_chan *dch = to_d350_chan(chan);
int ret = request_irq(dch->irq, d350_irq, IRQF_SHARED,
- dev_name(&dch->vc.chan.dev->device), dch);
+ dma_chan_name(&dch->vc.chan), dch);
if (!ret)
writel_relaxed(CH_INTREN_DONE | CH_INTREN_ERR, dch->base + CH_INTREN);
@@ -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) {
@@ -678,7 +678,7 @@ static int loongson2_cmc_dma_probe(struct platform_device *pdev)
return lchan->irq;
ret = devm_request_irq(dev, lchan->irq, loongson2_cmc_dma_chan_irq, IRQF_SHARED,
- dev_name(chan2dev(lchan)), lchan);
+ dma_chan_name(&lchan->vchan.chan), lchan);
if (ret)
return ret;
}
@@ -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),
@@ -1668,7 +1668,7 @@ static int stm32_dma_probe(struct platform_device *pdev)
ret = devm_request_irq(&pdev->dev, chan->irq,
stm32_dma_chan_irq, 0,
- dev_name(chan2dev(chan)), chan);
+ dma_chan_name(&chan->vchan.chan), chan);
if (ret) {
dev_err(&pdev->dev,
"request_irq failed with err %d channel %d\n",
@@ -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) {
@@ -1892,10 +1892,10 @@ static int stm32_dma3_probe(struct platform_device *pdev)
chan->irq = ret;
ret = devm_request_irq(&pdev->dev, chan->irq, stm32_dma3_chan_irq, 0,
- dev_name(chan2dev(chan)), chan);
+ dma_chan_name(&chan->vchan.chan), chan);
if (ret) {
dev_err_probe(&pdev->dev, ret, "Failed to request channel %s IRQ\n",
- dev_name(chan2dev(chan)));
+ dma_chan_name(&chan->vchan.chan));
goto err_clk_disable;
}
}
@@ -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(