Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Missing tasklet_kill() in driver removal path leads to Use-After-Free. - [High] Devres pool leak in error path of channel resource allocation. -- commit 46c4d3dece5bd1126d02c64f04aca18a9510a860 Author: Frank Li <[email protected]> dmaengine: use dma_chan_name() helper to get per-channel device name This patch updates several DMA engine drivers to use the dma_chan_name() helper instead of open-coding the channel name logic. This improves consistency and simplifies the code across the subsystem. [ ... ] > 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 is a pre-existing issue, but while reviewing this file's DMA resource management, I noticed a potential use-after-free during driver unbind. In mmp_pdma_chan_init(), a tasklet is initialized for each DMA channel using tasklet_setup(&chan->tasklet, dma_do_tasklet), and the channel structures are allocated via devm_kzalloc(). However, mmp_pdma_remove() unregisters the DMA engine but does not call tasklet_kill() to cancel pending tasklets. Because there is no explicit teardown of the tasklets, will pending or executing tasklets continue running after devres automatically frees the channel memory, resulting in a use-after-free if the driver is unbound while a tasklet is scheduled? [ ... ] > diff --git a/drivers/dma/stm32/stm32-mdma.c b/drivers/dma/stm32/stm32-mdma.c > index e3bbdc9ee36ed..c274638e919c2 100644 > --- a/drivers/dma/stm32/stm32-mdma.c > +++ b/drivers/dma/stm32/stm32-mdma.c > @@ -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), [Severity: High] This isn't a bug introduced by this patch, but looking at the error handling path immediately following this pool creation, is there a devres leak here? When dmam_pool_create() is called, it binds the devres release action to the DMA controller device (c->device->dev), rather than the channel lifecycle. If the subsequent pm_runtime_resume_and_get() call or stm32_mdma_disable_chan() fail, stm32_mdma_alloc_chan_resources() returns an error without destroying the newly allocated pool via dmam_pool_destroy(). Since the DMA engine core assumes the allocation function cleans up after itself on failure, will repeated allocation failures endlessly leak devres pool entries and memory on the controller device until the driver is unloaded? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
