* Re: [PATCH] dmaengine: pl330: fix autosuspend cleanup during removal
2026-08-08 9:44 [PATCH] dmaengine: pl330: fix autosuspend cleanup during removal Guangshuo Li
@ 2026-08-08 10:03 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-08-08 10:03 UTC (permalink / raw)
To: Guangshuo Li; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Pre-existing issues:
- [High] The IRQ handler accesses hardware registers unconditionally without checking if the device is active, risking a kernel panic during suspend.
- [High] Dynamically allocated `dma_pl330_desc` descriptors in the `desc_pool` are leaked when the driver unbinds.
- [High] The `peripherals` array allocated in `pl330_probe()` is never freed during device removal, causing a memory leak.
- [High] A debugfs file is created in the debugfs root but never removed, leading to a Use-After-Free upon unbind.
- [High] The IRQ handler is registered before the driver's synchronization primitives and tasklets are initialized, risking a kernel panic.
- [High] Casting an offset byte array pointer to `__le32 *` and dereferencing it causes an unaligned memory access.
--
--- Patch [1]: [PATCH] dmaengine: pl330: fix autosuspend cleanup during removal ---
commit e8b4012da1e87845c5bb71ff8b0d6b5e60fae048
Author: Guangshuo Li <lgs201920130244@gmail.com>
dmaengine: pl330: fix autosuspend cleanup during removal
pl330_probe() calls pm_runtime_use_autosuspend(), but pl330_remove()
does not call the matching pm_runtime_dont_use_autosuspend() when
removing the device. If the autosuspend delay is set to a negative value
while autosuspend is enabled, the runtime PM core increments usage_count
to prevent runtime suspend.
Cc: stable@vger.kernel.org
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 25ba84b187041..8baadd1047193 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -3204,6 +3204,7 @@ static void pl330_remove(struct amba_device *adev)
> struct dma_pl330_chan *pch, *_p;
> int i, irq;
>
> + pm_runtime_dont_use_autosuspend(&adev->dev);
> pm_runtime_get_noresume(pl330->ddma.dev);
>
> if (adev->dev.of_node)
[Severity: High]
This isn't a bug introduced by this patch, but does the IRQ handler access
hardware registers unconditionally without checking if the device is active?
drivers/dma/pl330.c:pl330_irq_handler() {
if (pl330_update(data))
return IRQ_HANDLED;
...
}
drivers/dma/pl330.c:pl330_update() {
...
val = readl(regs + FSM) & 0x1;
...
}
If a spurious interrupt occurs or an interrupt is in-flight while the device is
runtime suspended, pl330_update() unconditionally reads the FSM register. Since
the device clock is disabled during suspend, could accessing unclocked registers
result in an asynchronous bus fault on ARM architectures, leading to a system
crash?
[Severity: High]
This is a pre-existing issue, but are the dynamically allocated dma_pl330_desc
descriptors in the desc_pool leaked when the driver unbinds?
drivers/dma/pl330.c:add_desc() {
...
desc = kzalloc_objs(*desc, count, flg);
...
}
During probe and normal operation, add_desc() dynamically allocates batches
of descriptors. However, the driver's teardown functions pl330_remove() and
pl330_del() completely fail to iterate over the desc_pool or the channel lists
to free these dynamically allocated descriptors:
drivers/dma/pl330.c:pl330_del() {
...
dmac_free_threads(pl330);
dma_free_attrs(pl330->ddma.dev, ...);
}
Does this cause a permanent memory leak of all descriptors each time the device
is unbound?
[Severity: High]
This is a pre-existing issue, but is the peripherals array leaked during
device removal?
drivers/dma/pl330.c:pl330_probe() {
...
pl330->peripherals = kzalloc_objs(*pch, num_chan);
if (!pl330->peripherals) {
...
}
In pl330_probe(), memory for the pl330->peripherals array is dynamically
allocated using kzalloc_objs(). However, neither the teardown loop in
pl330_remove() nor the cleanup function pl330_del() calls
kfree(pl330->peripherals).
Since the allocation is not managed via devres, is the array permanently leaked
upon driver unbind?
[Severity: High]
This is a pre-existing issue, but does a debugfs file creation lead to a
Use-After-Free upon unbind?
drivers/dma/pl330.c:init_pl330_debugfs() {
debugfs_create_file(dev_name(pl330->ddma.dev),
S_IFREG | 0444, NULL, pl330,
&pl330_debugfs_fops);
}
The debugfs file is created in the debugfs root but never removed during
pl330_remove(). When the device unbinds, devres frees the pl330 struct, but
the debugfs file remains.
If userspace reads the file afterwards, does it trigger a Use-After-Free kernel
panic?
[Severity: High]
This is a pre-existing issue, but is the IRQ handler registered before the
driver's synchronization primitives and tasklets are initialized?
drivers/dma/pl330.c:pl330_probe() {
...
ret = devm_request_irq(&adev->dev, irq, pl330_irq_handler, 0,
dev_name(&adev->dev), pl330);
...
ret = pl330_add(pl330);
...
}
The hardware is taken out of reset and its IRQ is immediately requested.
However, the driver's primary spinlock and tasklet are not initialized until
pl330_add() runs later in the probe sequence:
drivers/dma/pl330.c:pl330_add() {
...
spin_lock_init(&pl330->lock);
...
}
If an interrupt fires during this window (e.g., a pending interrupt from before
kexec), pl330_irq_handler() will lock an uninitialized spinlock and schedule an
uninitialized tasklet. Does this risk a kernel panic?
[Severity: High]
This is a pre-existing issue, but does casting an offset byte array pointer
to __le32 * and dereferencing it cause an unaligned memory access?
drivers/dma/pl330.c:_execute_DBGINSN() {
...
val = le32_to_cpu(*((__le32 *)&insn[2]));
...
}
The code accesses the 3rd byte of the insn byte array. Because insn is a 6-byte
array allocated on the stack (e.g., in _stop()), &insn[2] is offset by 2 bytes
from the base pointer.
Does dereferencing this unaligned pointer as a 32-bit value violate C alignment
rules and trigger an alignment fault on architectures that do not support
unaligned accesses in hardware?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808094452.2752443-1-lgs201920130244@gmail.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread