* [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs
@ 2026-08-19 22:35 Linus Walleij
2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Linus Walleij @ 2026-08-19 22:35 UTC (permalink / raw)
To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot
This series fixes four pre-existing DMA40 bugs found while reviewing the
Ux500 LCLA SRAM power-domain conversion.
The fixes cover failed hardware-start cleanup, probe error unwinding for
runtime PM and IRQ registration, and validation of logical channel indexes
derived from dev_type before writing into lookup_log_chans.
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Linus Walleij (4):
dmaengine: ste_dma40: Fix failed start cleanup
dmaengine: ste_dma40: Fix probe runtime PM disable
dmaengine: ste_dma40: Fix probe IRQ leak
dmaengine: ste_dma40: Fix logical channel bounds check
drivers/dma/ste_dma40.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
---
base-commit: 0d995da5fb97e8c312834575604d4423eb6225b7
change-id: 20260820-dma40-fixes-b99af66002bf
Best regards,
--
Linus Walleij <linusw@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup 2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij @ 2026-08-19 22:35 ` Linus Walleij 2026-08-19 22:46 ` sashiko-bot 2026-08-19 22:35 ` [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Linus Walleij @ 2026-08-19 22:35 UTC (permalink / raw) To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot If d40_start() fails after a queued descriptor has been moved to the active list, d40_queue_start() currently returns NULL without unwinding the descriptor state or clearing the channel busy flag. Fix this pre-existing error path by removing and freeing the descriptor, clearing the busy flag and returning the error pointer to distinguish the failure from the no-work case. Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/ Assisted-by: Codex:gpt-5-5 Signed-off-by: Linus Walleij <linusw@kernel.org> --- drivers/dma/ste_dma40.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index 0d9ffa3e2663..d5837aa0a789 100644 --- a/drivers/dma/ste_dma40.c +++ b/drivers/dma/ste_dma40.c @@ -1531,8 +1531,12 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c) /* Start dma job */ err = d40_start(d40c); - if (err) - return NULL; + if (err) { + d40_desc_remove(d40d); + d40_desc_free(d40c, d40d); + d40c->busy = false; + return ERR_PTR(err); + } } return d40d; -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup 2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij @ 2026-08-19 22:46 ` sashiko-bot 0 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-08-19 22:46 UTC (permalink / raw) To: Linus Walleij; +Cc: vkoul, dmaengine, Frank.Li Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Returning ERR_PTR(err) from d40_queue_start() introduces a PM runtime reference leak and breaks caller logic in dma_tc_handle(). - [High] Freeing a submitted descriptor directly on error bypasses the DMA framework, causing the client to hang. -- --- Patch [1]: [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup --- commit 0baedf8f0f8ca2b28e1d6d6a6bce21f2db82302b Author: Linus Walleij <linusw@kernel.org> Date: Wed Aug 19 22:35:57 2026 +0000 dmaengine: ste_dma40: Fix failed start cleanup If d40_start() fails after a queued descriptor has been moved to the active list, d40_queue_start() currently returns NULL without unwinding the descriptor state or clearing the channel busy flag. This commit attempts to fix the error path by freeing the descriptor and returning an error pointer. Link: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/ > --- a/drivers/dma/ste_dma40.c > +++ b/drivers/dma/ste_dma40.c > @@ -1531,8 +1531,12 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c) > /* Start dma job */ > err = d40_start(d40c); > > - if (err) > - return NULL; > + if (err) { > + d40_desc_remove(d40d); > + d40_desc_free(d40c, d40d); [Severity: High] Does silently freeing the submitted descriptor here cause the DMA client to hang? The descriptor has already been submitted to the DMA framework, which means the client holds a cookie and expects a completion callback. Bypassing the framework and freeing the memory directly prevents the framework from completing the cookie, meaning the client will wait indefinitely for a callback that will never arrive. > + d40c->busy = false; > + return ERR_PTR(err); [Severity: High] Does returning ERR_PTR(err) here introduce a PM runtime reference leak? Earlier in d40_queue_start(), a PM runtime reference is acquired: if (d40d != NULL) { if (!d40c->busy) { d40c->busy = true; pm_runtime_get_sync(d40c->base->dev); } This error path returns ERR_PTR(err) and sets busy to false without dropping that newly acquired reference. Additionally, changing the return value breaks the caller logic in dma_tc_handle(), which explicitly checks for a NULL return to drop the PM reference: drivers/dma/ste_dma40.c:dma_tc_handle() { ... if (d40_queue_start(d40c) == NULL) { d40c->busy = false; pm_runtime_put_autosuspend(d40c->base->dev); } ... } Because ERR_PTR(err) is not NULL, this condition evaluates to false, skipping pm_runtime_put_autosuspend() and leaking the reference. > + } > } > > return d40d; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org?part=1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable 2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij 2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij @ 2026-08-19 22:35 ` Linus Walleij 2026-08-19 22:51 ` sashiko-bot 2026-08-19 22:35 ` [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij 2026-08-19 22:36 ` [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij 3 siblings, 1 reply; 9+ messages in thread From: Linus Walleij @ 2026-08-19 22:35 UTC (permalink / raw) To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot d40_probe() can jump to destroy_cache before runtime PM has been enabled for the DMA controller device. Calling pm_runtime_disable() on that path increments disable_depth, which can leave runtime PM disabled after a later successful deferred probe. Track whether this probe attempt enabled runtime PM before disabling it on the error path. Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/ Assisted-by: Codex:gpt-5-5 Signed-off-by: Linus Walleij <linusw@kernel.org> --- drivers/dma/ste_dma40.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index d5837aa0a789..0c8ff299c2c2 100644 --- a/drivers/dma/ste_dma40.c +++ b/drivers/dma/ste_dma40.c @@ -3502,6 +3502,7 @@ static int __init d40_probe(struct platform_device *pdev) struct resource *res; struct resource res_lcpa; int num_reserved_chans; + bool runtime_pm_enabled = false; u32 val; int ret; @@ -3623,6 +3624,7 @@ static int __init d40_probe(struct platform_device *pdev) pm_runtime_mark_last_busy(base->dev); pm_runtime_set_active(base->dev); pm_runtime_enable(base->dev); + runtime_pm_enabled = true; ret = d40_dmaengine_init(base, num_reserved_chans); if (ret) @@ -3658,7 +3660,8 @@ static int __init d40_probe(struct platform_device *pdev) regulator_disable(base->lcpa_regulator); regulator_put(base->lcpa_regulator); } - pm_runtime_disable(base->dev); + if (runtime_pm_enabled) + pm_runtime_disable(base->dev); report_failure: d40_err(dev, "probe failed\n"); -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable 2026-08-19 22:35 ` [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij @ 2026-08-19 22:51 ` sashiko-bot 0 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-08-19 22:51 UTC (permalink / raw) To: Linus Walleij; +Cc: vkoul, Frank.Li, dmaengine Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: New issues: - [Medium] The commit message claims to fix a bug regarding a 'later successful deferred probe', but deferred probe is structurally impossible for this driver. Pre-existing issues: - [High] Calling `free_pages()` on an `ioremap` mapped address in the probe error path. - [High] IRQ handler accesses hardware registers without checking runtime PM state. - [High] Passing a raw page count instead of an allocation order to `__get_free_pages()` and `free_pages()`. -- --- Patch [2]: [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable --- commit 724a6133520581f191bdd536bd3123c1d0c353fb Author: Linus Walleij <linusw@kernel.org> dmaengine: ste_dma40: Fix probe runtime PM disable d40_probe() can jump to destroy_cache before runtime PM has been enabled for the DMA controller device. Calling pm_runtime_disable() on that path increments disable_depth, which can leave runtime PM disabled after a later successful deferred probe. Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/ Assisted-by: Codex:gpt-5-5 Signed-off-by: Linus Walleij <linusw@kernel.org> > d40_probe() can jump to destroy_cache before runtime PM has been enabled > for the DMA controller device. Calling pm_runtime_disable() on that path > increments disable_depth, which can leave runtime PM disabled after a later > successful deferred probe. [Severity: Medium] Is this description accurate regarding deferred probe? The stedma40 driver is registered using platform_driver_probe() via stedma40_init(), which sets drv->prevent_deferred_probe = true and prevents any future binds. A later successful deferred probe appears to be impossible here, even though fixing the unbalanced disable_depth is still semantically correct. > diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c > index d5837aa0a789a..0c8ff299c2c2d 100644 > --- a/drivers/dma/ste_dma40.c > +++ b/drivers/dma/ste_dma40.c [ ... ] > @@ -3658,7 +3660,8 @@ static int __init d40_probe(struct platform_device *pdev) > regulator_disable(base->lcpa_regulator); > regulator_put(base->lcpa_regulator); > } [Severity: High] This is a pre-existing issue, but does the error path in destroy_cache call free_pages() on an ioremap address? If base->plat_data->use_esram_lcla is true, memory is mapped using devm_ioremap() and assigned to base->lcla_pool.base. Just above this regulator cleanup in destroy_cache, the code does: if (!base->lcla_pool.base_unaligned && base->lcla_pool.base) free_pages((unsigned long)base->lcla_pool.base, base->lcla_pool.pages); Calling virt_to_page() on an ioremap address yields a garbage pointer, which would cause a crash when __free_pages() dereferences it. > - pm_runtime_disable(base->dev); > + if (runtime_pm_enabled) > + pm_runtime_disable(base->dev); > > report_failure: > d40_err(dev, "probe failed\n"); [Severity: High] This is a pre-existing issue, but does the interrupt handler access hardware registers without checking the runtime PM state? A spurious interrupt could fire while the device is in runtime suspend. The IRQ handler d40_handle_interrupt() unconditionally accesses the PrimeCell registers: drivers/dma/ste_dma40.c:d40_handle_interrupt() { ... regs[i] = readl(base->virtbase + il[i].src); ... } Since dma40_runtime_suspend() turns off the GCC clock, reading the unclocked registers without calling pm_runtime_get_if_active() could cause a bus fault (external abort). [Severity: High] This is a pre-existing issue, but does d40_lcla_allocate() pass a raw page count instead of an allocation order? During allocation, base->lcla_pool.pages is calculated as the absolute number of pages needed: drivers/dma/ste_dma40.c:d40_lcla_allocate() { ... base->lcla_pool.pages = SZ_1K * base->num_phy_chans / PAGE_SIZE; ... page_list[i] = __get_free_pages(GFP_KERNEL, base->lcla_pool.pages); ... } This raw count is then erroneously passed as the order parameter to __get_free_pages(). For example, an order of 8 would allocate 256 pages instead of the 8 pages actually needed, leading to wasted memory or potential allocation failures. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org?part=2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak 2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij 2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij 2026-08-19 22:35 ` [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij @ 2026-08-19 22:35 ` Linus Walleij 2026-08-19 22:52 ` sashiko-bot 2026-08-19 22:36 ` [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij 3 siblings, 1 reply; 9+ messages in thread From: Linus Walleij @ 2026-08-19 22:35 UTC (permalink / raw) To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot d40_probe() registers the hardware interrupt before several later probe steps that can fail. Those error paths jump to destroy_cache without freeing the IRQ, leaving the handler registered after probe resources have been released. Track successful IRQ registration and free the IRQ on later probe failure. Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/ Assisted-by: Codex:gpt-5-5 Signed-off-by: Linus Walleij <linusw@kernel.org> --- drivers/dma/ste_dma40.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index 0c8ff299c2c2..a723133d096e 100644 --- a/drivers/dma/ste_dma40.c +++ b/drivers/dma/ste_dma40.c @@ -3503,6 +3503,7 @@ static int __init d40_probe(struct platform_device *pdev) struct resource res_lcpa; int num_reserved_chans; bool runtime_pm_enabled = false; + bool irq_requested = false; u32 val; int ret; @@ -3595,6 +3596,7 @@ static int __init d40_probe(struct platform_device *pdev) d40_err(dev, "No IRQ defined\n"); goto destroy_cache; } + irq_requested = true; if (base->plat_data->use_esram_lcla) { @@ -3660,6 +3662,8 @@ static int __init d40_probe(struct platform_device *pdev) regulator_disable(base->lcpa_regulator); regulator_put(base->lcpa_regulator); } + if (irq_requested) + free_irq(base->irq, base); if (runtime_pm_enabled) pm_runtime_disable(base->dev); -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak 2026-08-19 22:35 ` [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij @ 2026-08-19 22:52 ` sashiko-bot 0 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-08-19 22:52 UTC (permalink / raw) To: Linus Walleij; +Cc: dmaengine, vkoul, Frank.Li Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: Pre-existing issues: - [High] The probe error path incorrectly calls free_pages() on an ioremap pointer when ESRAM is used for LCLA. - [High] The DMA engine devices are registered using devres (dmaenginem_async_device_register), but core memory resources and the interrupt handler are manually freed in the probe error path, risking Use-After-Free. - [High] d40_lcla_allocate() passes a calculated number of pages instead of a logarithmic order to __get_free_pages(), leading to severe memory overallocation. - [High] The DMA engine is registered and exposed to other subsystems before hardware initialization is complete. - [High] Out-of-bounds write in the global dma40_memcpy_channels array via unverified device tree input. -- --- Patch [3]: [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak --- commit 5f3ddaa41f4f5eab5f519e10436cd8e82088831f Author: Linus Walleij <linusw@kernel.org> dmaengine: ste_dma40: Fix probe IRQ leak d40_probe() registers the hardware interrupt before several later probe steps that can fail. Those error paths jump to destroy_cache without freeing the IRQ, leaving the handler registered after probe resources have been released. This patch tracks successful IRQ registration and frees the IRQ on later probe failure. Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/ > diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c > index 0c8ff299c2c2d..a723133d096ec 100644 > --- a/drivers/dma/ste_dma40.c > +++ b/drivers/dma/ste_dma40.c > @@ -3503,6 +3503,7 @@ static int __init d40_probe(struct platform_device *pdev) > struct resource res_lcpa; > int num_reserved_chans; > bool runtime_pm_enabled = false; > + bool irq_requested = false; > u32 val; > int ret; > [ ... ] > @@ -3595,6 +3596,7 @@ static int __init d40_probe(struct platform_device *pdev) > d40_err(dev, "No IRQ defined\n"); > goto destroy_cache; > } > + irq_requested = true; > > if (base->plat_data->use_esram_lcla) { > [ ... ] > @@ -3660,6 +3662,8 @@ static int __init d40_probe(struct platform_device *pdev) > regulator_disable(base->lcpa_regulator); > regulator_put(base->lcpa_regulator); > } > + if (irq_requested) > + free_irq(base->irq, base); [Severity: High] This isn't a bug introduced by this patch, but does the destroy_cache error path call free_pages() on an ioremap pointer when ESRAM is used for LCLA? In d40_probe(), if base->plat_data->use_esram_lcla is true, base->lcla_pool.base is initialized with devm_ioremap(): drivers/dma/ste_dma40.c:d40_probe() { ... if (base->plat_data->use_esram_lcla) { ... base->lcla_pool.base = devm_ioremap(dev, res->start, resource_size(res)); ... } However, in the destroy_cache error path, it seems to be freed using free_pages(): drivers/dma/ste_dma40.c:d40_probe() { ... destroy_cache: ... if (!base->lcla_pool.base_unaligned && base->lcla_pool.base) free_pages((unsigned long)base->lcla_pool.base, base->lcla_pool.pages); ... } Would passing this MMIO address to the page allocator cause a VM_BUG_ON or memory corruption? [Severity: High] This isn't a bug introduced by this patch, but does the error path risk a use-after-free by manually freeing core memory resources while the DMA engine devices are still registered? In d40_probe(), the DMA engine devices are registered using d40_dmaengine_init(): drivers/dma/ste_dma40.c:d40_probe() { ... ret = d40_dmaengine_init(base, num_reserved_chans); if (ret) goto destroy_cache; ... } If a failure occurs after this point, the destroy_cache path manually frees lcla_pool and the IRQ handler. Since d40_dmaengine_init() likely uses devres to register the DMA devices, they remain globally visible until the probe function returns and devres unwinds. Could this allow a concurrent client to initiate a DMA transfer using freed resources? [Severity: High] This is a pre-existing issue, but is the DMA engine registered and exposed to other subsystems before hardware initialization is complete? In d40_probe(), the DMA devices are registered via d40_dmaengine_init() before d40_hw_init() is called: drivers/dma/ste_dma40.c:d40_probe() { ... ret = d40_dmaengine_init(base, num_reserved_chans); if (ret) goto destroy_cache; ... d40_hw_init(base); ... } If a client requests a channel and starts a transfer before d40_hw_init() finishes, could this lead to a bus lockup or corrupted transfers? [Severity: High] This is a pre-existing issue, but does d40_lcla_allocate() pass a page count instead of a logarithmic order to __get_free_pages()? In d40_lcla_allocate(), the required number of pages is calculated and stored in base->lcla_pool.pages: drivers/dma/ste_dma40.c:d40_lcla_allocate() { ... base->lcla_pool.pages = SZ_1K * base->num_phy_chans / PAGE_SIZE; ... page_list[i] = __get_free_pages(GFP_KERNEL, base->lcla_pool.pages); ... } If SZ_1K * base->num_phy_chans / PAGE_SIZE evaluates to 8, wouldn't passing 8 as the order request 2^8 (256) pages instead of 8, leading to massive memory overallocation? [Severity: High] This isn't a bug introduced by this patch, but is there an out-of-bounds write when parsing memcpy-channels from the device tree? The global array dma40_memcpy_channels is sized to exactly 6 elements: drivers/dma/ste_dma40.c: static int dma40_memcpy_channels[] = { DB8500_DMA_MEMCPY_EV_0, ... DB8500_DMA_MEMCPY_EV_5, }; In d40_of_probe(), the length is checked against D40_MEMCPY_MAX_CHANS (which is 8), and then of_property_read_u32_array() writes up to 8 elements: drivers/dma/ste_dma40.c:d40_of_probe() { ... if (num_memcpy > D40_MEMCPY_MAX_CHANS || num_memcpy <= 0) { ... } ... of_property_read_u32_array(np, "memcpy-channels", dma40_memcpy_channels, num_memcpy); ... } Could this overwrite adjacent kernel data if the device tree specifies 7 or 8 elements? > if (runtime_pm_enabled) > pm_runtime_disable(base->dev); -- Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org?part=3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check 2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij ` (2 preceding siblings ...) 2026-08-19 22:35 ` [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij @ 2026-08-19 22:36 ` Linus Walleij 2026-08-19 22:50 ` sashiko-bot 3 siblings, 1 reply; 9+ messages in thread From: Linus Walleij @ 2026-08-19 22:36 UTC (permalink / raw) To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot d40_validate_conf() checks the raw dev_type against num_log_chans, but d40_allocate_channel() derives the lookup_log_chans index as either 2 * dev_type or 2 * dev_type + 1. Validate the dev_type against the derived logical channel index limit so channel allocation cannot write past lookup_log_chans. Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/ Assisted-by: Codex:gpt-5-5 Signed-off-by: Linus Walleij <linusw@kernel.org> --- drivers/dma/ste_dma40.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index a723133d096e..eda3f91741ed 100644 --- a/drivers/dma/ste_dma40.c +++ b/drivers/dma/ste_dma40.c @@ -1727,15 +1727,26 @@ static int d40_validate_conf(struct d40_chan *d40c, { int res = 0; bool is_log = conf->mode == STEDMA40_MODE_LOGICAL; + bool invalid_dev_type = conf->dev_type < 0; if (!conf->dir) { chan_err(d40c, "Invalid direction.\n"); res = -EINVAL; } - if ((is_log && conf->dev_type > d40c->base->num_log_chans) || - (!is_log && conf->dev_type > d40c->base->num_phy_chans) || - (conf->dev_type < 0)) { + if (!invalid_dev_type && is_log) { + int max_dev_type; + + if (conf->dir == DMA_DEV_TO_MEM) + max_dev_type = DIV_ROUND_UP(d40c->base->num_log_chans, 2); + else + max_dev_type = d40c->base->num_log_chans / 2; + + invalid_dev_type = conf->dev_type >= max_dev_type; + } + + if (invalid_dev_type || + (!is_log && conf->dev_type > d40c->base->num_phy_chans)) { chan_err(d40c, "Invalid device type (%d)\n", conf->dev_type); res = -EINVAL; } -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check 2026-08-19 22:36 ` [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij @ 2026-08-19 22:50 ` sashiko-bot 0 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-08-19 22:50 UTC (permalink / raw) To: Linus Walleij; +Cc: vkoul, dmaengine, Frank.Li Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: Pre-existing issues: - [High] Out-of-bounds access on the `phys` array and potentially arbitrary MMIO writes during channel allocation due to unconstrained `event_group`. - [High] The out-of-bounds write fix for `lookup_log_chans` is bypassed for memcpy channels configured via the Device Tree. - [High] Global buffer overflow when reading `memcpy-channels` from Device Tree. -- --- Patch [4]: [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check --- commit 53c98a760123837b647fa917a72020a724b5f32f Author: Linus Walleij <linusw@kernel.org> dmaengine: ste_dma40: Fix logical channel bounds check d40_validate_conf() checks the raw dev_type against num_log_chans, but d40_allocate_channel() derives the lookup_log_chans index as either 2 * dev_type or 2 * dev_type + 1. Validate the dev_type against the derived logical channel index limit so channel allocation cannot write past lookup_log_chans. Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/ > diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c > index a723133d096ec..eda3f91741ed5 100644 > --- a/drivers/dma/ste_dma40.c > +++ b/drivers/dma/ste_dma40.c > @@ -1727,15 +1727,26 @@ static int d40_validate_conf(struct d40_chan *d40c, > { > int res = 0; > bool is_log = conf->mode == STEDMA40_MODE_LOGICAL; > + bool invalid_dev_type = conf->dev_type < 0; > > if (!conf->dir) { > chan_err(d40c, "Invalid direction.\n"); > res = -EINVAL; > } > > - if ((is_log && conf->dev_type > d40c->base->num_log_chans) || > - (!is_log && conf->dev_type > d40c->base->num_phy_chans) || > - (conf->dev_type < 0)) { > + if (!invalid_dev_type && is_log) { > + int max_dev_type; > + > + if (conf->dir == DMA_DEV_TO_MEM) > + max_dev_type = DIV_ROUND_UP(d40c->base->num_log_chans, 2); > + else > + max_dev_type = d40c->base->num_log_chans / 2; > + > + invalid_dev_type = conf->dev_type >= max_dev_type; > + } [Severity: High] This is a pre-existing issue, but does this bounds check fully protect the phys array during channel allocation? In d40_allocate_channel(), phy_num is calculated as j + event_group * 2, where event_group is derived from dev_type / 16. If num_log_chans is heavily scaled compared to num_phy_chans, could an allowed logical dev_type produce an event_group that causes event_group * 2 to greatly exceed the physical channel count? drivers/dma/ste_dma40.c:d40_allocate_channel() { ... for (i = phy_num; i < phy_num + 2; i++) { if (d40_alloc_mask_set(&phys[i], ... ... } Could this cause out-of-bounds accesses on the phys array, as well as arbitrary MMIO writes later in __d40_set_prio_rt() which uses the unvalidated group multiplier? drivers/dma/ste_dma40.c:__d40_set_prio_rt() { ... writel(bit, d40c->base->virtbase + rtreg + group * 4); ... } [Severity: High] This is also a pre-existing issue, but is the validation in d40_validate_conf() bypassed for memcpy channels configured via the Device Tree? In the fallback configuration path d40_config_memcpy(), the driver assigns dev_type directly from the dma40_memcpy_channels array without calling d40_validate_conf(): drivers/dma/ste_dma40.c:d40_config_memcpy() { ... d40c->dma_cfg.dev_type = dma40_memcpy_channels[d40c->chan.chan_id]; ... } If the device tree supplies an arbitrarily large ID for a memcpy channel via the memcpy-channels property, could this unvalidated dev_type cause an out-of-bounds write into lookup_log_chans during allocation, subverting this patch's fix? drivers/dma/ste_dma40.c:d40_allocate_channel() { ... d40c->base->lookup_log_chans[d40c->log_num] = d40c; ... } [Severity: High] Another pre-existing issue, but is there a risk of a global buffer overflow when reading memcpy-channels from the Device Tree? The dma40_memcpy_channels array is statically sized with exactly 6 elements: drivers/dma/ste_dma40.c: static int dma40_memcpy_channels[] = { DB8500_DMA_MEMCPY_EV_0, ... DB8500_DMA_MEMCPY_EV_5, }; However, in d40_probe(), the DT parsing logic reads memcpy-channels up to D40_MEMCPY_MAX_CHANS (which is 8) via of_property_read_u32_array(): drivers/dma/ste_dma40.c:d40_probe() { ... of_property_read_u32_array(np, "memcpy-channels", dma40_memcpy_channels, num_memcpy); ... } If a Device Tree blob provides 7 or 8 elements in the memcpy-channels property, will the driver write past the end of the 6-element array during initialization? > + > + if (invalid_dev_type || > + (!is_log && conf->dev_type > d40c->base->num_phy_chans)) { > chan_err(d40c, "Invalid device type (%d)\n", conf->dev_type); > res = -EINVAL; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org?part=4 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-19 22:52 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij 2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij 2026-08-19 22:46 ` sashiko-bot 2026-08-19 22:35 ` [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij 2026-08-19 22:51 ` sashiko-bot 2026-08-19 22:35 ` [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij 2026-08-19 22:52 ` sashiko-bot 2026-08-19 22:36 ` [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij 2026-08-19 22:50 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox