* [PATCH v5 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up [not found] <20260512121219.216159-1-claudiu.beznea.uj@bp.renesas.com> @ 2026-05-12 12:12 ` Claudiu Beznea 2026-05-12 20:28 ` Frank Li 2026-05-12 12:12 ` [PATCH v5 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() Claudiu Beznea 1 sibling, 1 reply; 5+ messages in thread From: Claudiu Beznea @ 2026-05-12 12:12 UTC (permalink / raw) To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz, prabhakar.mahadev-lad.rj, p.zabel, geert+renesas, fabrizio.castro.jz, kuninori.morimoto.gx, long.luu.ur Cc: claudiu.beznea, dmaengine, linux-kernel, linux-sound, linux-renesas-soc, Claudiu Beznea, stable Once the interrupt is requested, the interrupt handler may run immediately. Since the IRQ handler can access channel->ch_base, which is initialized only after requesting the IRQ, this may lead to invalid memory access. Likewise, the IRQ thread may access uninitialized data (the ld_free, ld_queue, and ld_active lists), which may also lead to issues. Request the interrupts only after everything is set up. To keep the error path simpler, use dmam_alloc_coherent() instead of dma_alloc_coherent(). Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC") Cc: stable@vger.kernel.org Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- Changes in v5: - none Changes in v4: - none, this patch is new drivers/dma/sh/rz-dmac.c | 88 +++++++++++++++------------------------- 1 file changed, 33 insertions(+), 55 deletions(-) diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 625ff29024de..9f206a33dcc6 100644 --- a/drivers/dma/sh/rz-dmac.c +++ b/drivers/dma/sh/rz-dmac.c @@ -981,25 +981,6 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, channel->index = index; channel->mid_rid = -EINVAL; - /* Request the channel interrupt. */ - scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); - irq = platform_get_irq_byname(pdev, pdev_irqname); - if (irq < 0) - return irq; - - irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", - dev_name(dmac->dev), index); - if (!irqname) - return -ENOMEM; - - ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, - rz_dmac_irq_handler_thread, 0, - irqname, channel); - if (ret) { - dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); - return ret; - } - /* Set io base address for each channel */ if (index < 8) { channel->ch_base = dmac->base + CHANNEL_0_7_OFFSET + @@ -1012,9 +993,9 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, } /* Allocate descriptors */ - lmdesc = dma_alloc_coherent(&pdev->dev, - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, - &channel->lmdesc.base_dma, GFP_KERNEL); + lmdesc = dmam_alloc_coherent(&pdev->dev, + sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, + &channel->lmdesc.base_dma, GFP_KERNEL); if (!lmdesc) { dev_err(&pdev->dev, "Can't allocate memory (lmdesc)\n"); return -ENOMEM; @@ -1030,7 +1011,24 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, INIT_LIST_HEAD(&channel->ld_free); INIT_LIST_HEAD(&channel->ld_active); - return 0; + /* Request the channel interrupt. */ + scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); + irq = platform_get_irq_byname(pdev, pdev_irqname); + if (irq < 0) + return irq; + + irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", + dev_name(dmac->dev), index); + if (!irqname) + return -ENOMEM; + + ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, + rz_dmac_irq_handler_thread, 0, + irqname, channel); + if (ret) + dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); + + return ret; } static void rz_dmac_put_device(void *_dev) @@ -1099,7 +1097,6 @@ static int rz_dmac_probe(struct platform_device *pdev) const char *irqname = "error"; struct dma_device *engine; struct rz_dmac *dmac; - int channel_num; int ret; int irq; u8 i; @@ -1132,18 +1129,6 @@ static int rz_dmac_probe(struct platform_device *pdev) return PTR_ERR(dmac->ext_base); } - /* Register interrupt handler for error */ - irq = platform_get_irq_byname_optional(pdev, irqname); - if (irq > 0) { - ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0, - irqname, NULL); - if (ret) { - dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", - irq, ret); - return ret; - } - } - /* Initialize the channels. */ INIT_LIST_HEAD(&dmac->engine.channels); @@ -1169,6 +1154,18 @@ static int rz_dmac_probe(struct platform_device *pdev) goto err; } + /* Register interrupt handler for error */ + irq = platform_get_irq_byname_optional(pdev, irqname); + if (irq > 0) { + ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0, + irqname, NULL); + if (ret) { + dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", + irq, ret); + goto err; + } + } + /* Register the DMAC as a DMA provider for DT. */ ret = of_dma_controller_register(pdev->dev.of_node, rz_dmac_of_xlate, NULL); @@ -1210,16 +1207,6 @@ static int rz_dmac_probe(struct platform_device *pdev) dma_register_err: of_dma_controller_free(pdev->dev.of_node); err: - channel_num = i ? i - 1 : 0; - for (i = 0; i < channel_num; i++) { - struct rz_dmac_chan *channel = &dmac->channels[i]; - - dma_free_coherent(&pdev->dev, - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, - channel->lmdesc.base, - channel->lmdesc.base_dma); - } - reset_control_assert(dmac->rstc); err_pm_runtime_put: pm_runtime_put(&pdev->dev); @@ -1232,18 +1219,9 @@ static int rz_dmac_probe(struct platform_device *pdev) static void rz_dmac_remove(struct platform_device *pdev) { struct rz_dmac *dmac = platform_get_drvdata(pdev); - unsigned int i; dma_async_device_unregister(&dmac->engine); of_dma_controller_free(pdev->dev.of_node); - for (i = 0; i < dmac->n_channels; i++) { - struct rz_dmac_chan *channel = &dmac->channels[i]; - - dma_free_coherent(&pdev->dev, - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, - channel->lmdesc.base, - channel->lmdesc.base_dma); - } reset_control_assert(dmac->rstc); pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev); -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up 2026-05-12 12:12 ` [PATCH v5 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up Claudiu Beznea @ 2026-05-12 20:28 ` Frank Li 0 siblings, 0 replies; 5+ messages in thread From: Frank Li @ 2026-05-12 20:28 UTC (permalink / raw) To: Claudiu Beznea Cc: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz, prabhakar.mahadev-lad.rj, p.zabel, geert+renesas, fabrizio.castro.jz, kuninori.morimoto.gx, long.luu.ur, claudiu.beznea, dmaengine, linux-kernel, linux-sound, linux-renesas-soc, stable On Tue, May 12, 2026 at 03:12:02PM +0300, Claudiu Beznea wrote: > Once the interrupt is requested, the interrupt handler may run immediately. > Since the IRQ handler can access channel->ch_base, which is initialized > only after requesting the IRQ, this may lead to invalid memory access. > Likewise, the IRQ thread may access uninitialized data (the ld_free, > ld_queue, and ld_active lists), which may also lead to issues. > > Request the interrupts only after everything is set up. To keep the error > path simpler, use dmam_alloc_coherent() instead of dma_alloc_coherent(). > > Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC") > Cc: stable@vger.kernel.org > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- Reviewed-by: Frank Li <Frank.Li@nxp.com> > > Changes in v5: > - none > > Changes in v4: > - none, this patch is new > > drivers/dma/sh/rz-dmac.c | 88 +++++++++++++++------------------------- > 1 file changed, 33 insertions(+), 55 deletions(-) > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c > index 625ff29024de..9f206a33dcc6 100644 > --- a/drivers/dma/sh/rz-dmac.c > +++ b/drivers/dma/sh/rz-dmac.c > @@ -981,25 +981,6 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, > channel->index = index; > channel->mid_rid = -EINVAL; > > - /* Request the channel interrupt. */ > - scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); > - irq = platform_get_irq_byname(pdev, pdev_irqname); > - if (irq < 0) > - return irq; > - > - irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", > - dev_name(dmac->dev), index); > - if (!irqname) > - return -ENOMEM; > - > - ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, > - rz_dmac_irq_handler_thread, 0, > - irqname, channel); > - if (ret) { > - dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); > - return ret; > - } > - > /* Set io base address for each channel */ > if (index < 8) { > channel->ch_base = dmac->base + CHANNEL_0_7_OFFSET + > @@ -1012,9 +993,9 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, > } > > /* Allocate descriptors */ > - lmdesc = dma_alloc_coherent(&pdev->dev, > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > - &channel->lmdesc.base_dma, GFP_KERNEL); > + lmdesc = dmam_alloc_coherent(&pdev->dev, > + sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > + &channel->lmdesc.base_dma, GFP_KERNEL); > if (!lmdesc) { > dev_err(&pdev->dev, "Can't allocate memory (lmdesc)\n"); > return -ENOMEM; > @@ -1030,7 +1011,24 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, > INIT_LIST_HEAD(&channel->ld_free); > INIT_LIST_HEAD(&channel->ld_active); > > - return 0; > + /* Request the channel interrupt. */ > + scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); > + irq = platform_get_irq_byname(pdev, pdev_irqname); > + if (irq < 0) > + return irq; > + > + irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", > + dev_name(dmac->dev), index); > + if (!irqname) > + return -ENOMEM; > + > + ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, > + rz_dmac_irq_handler_thread, 0, > + irqname, channel); > + if (ret) > + dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); > + > + return ret; > } > > static void rz_dmac_put_device(void *_dev) > @@ -1099,7 +1097,6 @@ static int rz_dmac_probe(struct platform_device *pdev) > const char *irqname = "error"; > struct dma_device *engine; > struct rz_dmac *dmac; > - int channel_num; > int ret; > int irq; > u8 i; > @@ -1132,18 +1129,6 @@ static int rz_dmac_probe(struct platform_device *pdev) > return PTR_ERR(dmac->ext_base); > } > > - /* Register interrupt handler for error */ > - irq = platform_get_irq_byname_optional(pdev, irqname); > - if (irq > 0) { > - ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0, > - irqname, NULL); > - if (ret) { > - dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", > - irq, ret); > - return ret; > - } > - } > - > /* Initialize the channels. */ > INIT_LIST_HEAD(&dmac->engine.channels); > > @@ -1169,6 +1154,18 @@ static int rz_dmac_probe(struct platform_device *pdev) > goto err; > } > > + /* Register interrupt handler for error */ > + irq = platform_get_irq_byname_optional(pdev, irqname); > + if (irq > 0) { > + ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0, > + irqname, NULL); > + if (ret) { > + dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", > + irq, ret); > + goto err; > + } > + } > + > /* Register the DMAC as a DMA provider for DT. */ > ret = of_dma_controller_register(pdev->dev.of_node, rz_dmac_of_xlate, > NULL); > @@ -1210,16 +1207,6 @@ static int rz_dmac_probe(struct platform_device *pdev) > dma_register_err: > of_dma_controller_free(pdev->dev.of_node); > err: > - channel_num = i ? i - 1 : 0; > - for (i = 0; i < channel_num; i++) { > - struct rz_dmac_chan *channel = &dmac->channels[i]; > - > - dma_free_coherent(&pdev->dev, > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > - channel->lmdesc.base, > - channel->lmdesc.base_dma); > - } > - > reset_control_assert(dmac->rstc); > err_pm_runtime_put: > pm_runtime_put(&pdev->dev); > @@ -1232,18 +1219,9 @@ static int rz_dmac_probe(struct platform_device *pdev) > static void rz_dmac_remove(struct platform_device *pdev) > { > struct rz_dmac *dmac = platform_get_drvdata(pdev); > - unsigned int i; > > dma_async_device_unregister(&dmac->engine); > of_dma_controller_free(pdev->dev.of_node); > - for (i = 0; i < dmac->n_channels; i++) { > - struct rz_dmac_chan *channel = &dmac->channels[i]; > - > - dma_free_coherent(&pdev->dev, > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > - channel->lmdesc.base, > - channel->lmdesc.base_dma); > - } > reset_control_assert(dmac->rstc); > pm_runtime_put(&pdev->dev); > pm_runtime_disable(&pdev->dev); > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() [not found] <20260512121219.216159-1-claudiu.beznea.uj@bp.renesas.com> 2026-05-12 12:12 ` [PATCH v5 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up Claudiu Beznea @ 2026-05-12 12:12 ` Claudiu Beznea 2026-05-12 20:35 ` Frank Li 1 sibling, 1 reply; 5+ messages in thread From: Claudiu Beznea @ 2026-05-12 12:12 UTC (permalink / raw) To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz, prabhakar.mahadev-lad.rj, p.zabel, geert+renesas, fabrizio.castro.jz, kuninori.morimoto.gx, long.luu.ur Cc: claudiu.beznea, dmaengine, linux-kernel, linux-sound, linux-renesas-soc, Claudiu Beznea, stable The list passed as argument to list_first_entry() is expected to be not empty. Use list_first_entry_or_null() to avoid dereferencing invalid memory. Fixes: 21323b118c16 ("dmaengine: sh: rz-dmac: Add device_tx_status() callback") Cc: stable@vger.kernel.org Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- Changes in v5: - none Changes in v4: - none, this patch is new drivers/dma/sh/rz-dmac.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 9f206a33dcc6..6d80cb668957 100644 --- a/drivers/dma/sh/rz-dmac.c +++ b/drivers/dma/sh/rz-dmac.c @@ -723,8 +723,8 @@ static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel, u32 crla, crtb, i; /* Get current processing virtual descriptor */ - current_desc = list_first_entry(&channel->ld_active, - struct rz_dmac_desc, node); + current_desc = list_first_entry_or_null(&channel->ld_active, + struct rz_dmac_desc, node); if (!current_desc) return 0; -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() 2026-05-12 12:12 ` [PATCH v5 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() Claudiu Beznea @ 2026-05-12 20:35 ` Frank Li 2026-05-13 13:31 ` Claudiu Beznea 0 siblings, 1 reply; 5+ messages in thread From: Frank Li @ 2026-05-12 20:35 UTC (permalink / raw) To: Claudiu Beznea Cc: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz, prabhakar.mahadev-lad.rj, p.zabel, geert+renesas, fabrizio.castro.jz, kuninori.morimoto.gx, long.luu.ur, claudiu.beznea, dmaengine, linux-kernel, linux-sound, linux-renesas-soc, stable On Tue, May 12, 2026 at 03:12:03PM +0300, Claudiu Beznea wrote: > The list passed as argument to list_first_entry() is expected to be not > empty. Little confused, #define list_first_entry_or_null(ptr, type, member) ({ \ struct list_head *head__ = (ptr); \ struct list_head *pos__ = READ_ONCE(head__->next); \ pos__ != head__ ? list_entry(pos__, type, member) : NULL; \ }) both list passed to list_first_entry() or list_first_entry_or_null() must be not NULL. The return value is difference. Fix incorrect NULL check for list_first_entry() list_first_entry() does not return NULL when the list is empty, making the existing NULL check invalid. Use list_first_entry_or_null() instead. Frank > Use list_first_entry_or_null() to avoid dereferencing invalid > memory. > > Fixes: 21323b118c16 ("dmaengine: sh: rz-dmac: Add device_tx_status() callback") > Cc: stable@vger.kernel.org > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > > Changes in v5: > - none > > Changes in v4: > - none, this patch is new > > drivers/dma/sh/rz-dmac.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c > index 9f206a33dcc6..6d80cb668957 100644 > --- a/drivers/dma/sh/rz-dmac.c > +++ b/drivers/dma/sh/rz-dmac.c > @@ -723,8 +723,8 @@ static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel, > u32 crla, crtb, i; > > /* Get current processing virtual descriptor */ > - current_desc = list_first_entry(&channel->ld_active, > - struct rz_dmac_desc, node); > + current_desc = list_first_entry_or_null(&channel->ld_active, > + struct rz_dmac_desc, node); > if (!current_desc) > return 0; > > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() 2026-05-12 20:35 ` Frank Li @ 2026-05-13 13:31 ` Claudiu Beznea 0 siblings, 0 replies; 5+ messages in thread From: Claudiu Beznea @ 2026-05-13 13:31 UTC (permalink / raw) To: Frank Li, Claudiu Beznea Cc: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz, prabhakar.mahadev-lad.rj, p.zabel, geert+renesas, fabrizio.castro.jz, kuninori.morimoto.gx, long.luu.ur, dmaengine, linux-kernel, linux-sound, linux-renesas-soc, stable Hi, Frank, On 5/12/26 23:35, Frank Li wrote: > On Tue, May 12, 2026 at 03:12:03PM +0300, Claudiu Beznea wrote: >> The list passed as argument to list_first_entry() is expected to be not >> empty. > > Little confused, > > #define list_first_entry_or_null(ptr, type, member) ({ \ > struct list_head *head__ = (ptr); \ > struct list_head *pos__ = READ_ONCE(head__->next); \ > pos__ != head__ ? list_entry(pos__, type, member) : NULL; \ > }) > > > both list passed to list_first_entry() or list_first_entry_or_null() must > be not NULL. The intention was to to express that checking the pointer returned by list_first_entry() may lead to problems. I'll adjust the description to be more clear. -- Thank you, Claudiu ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-13 13:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260512121219.216159-1-claudiu.beznea.uj@bp.renesas.com>
2026-05-12 12:12 ` [PATCH v5 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up Claudiu Beznea
2026-05-12 20:28 ` Frank Li
2026-05-12 12:12 ` [PATCH v5 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() Claudiu Beznea
2026-05-12 20:35 ` Frank Li
2026-05-13 13:31 ` Claudiu Beznea
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox