Re: [PATCH v6 06/10] dmaengine: switchtec-dma: disable channels before freeing on registration failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Premature registration of chan_status_irq allows the ISR to execute before channels are initialized, causing NULL pointer dereferences.
- [High] Missing ioremap() return value check in switchtec_dma_create() can lead to a NULL pointer dereference.
--

commit 5a07655c2107e09316788ad5789b3b1db4682eda
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: disable channels before freeing on registration failure

This commit addresses an issue in switchtec_dma_create() where an error during
or after channel enumeration could leave the channel status IRQ active. It
introduces switchtec_dma_chans_disable() to free the IRQ before channels are
released and freed, preventing the IRQ from firing mid-teardown.

Link: https://lore.kernel.org/dmaengine/20260717223431.625EE1F000E9@smtp.kernel.org
Link: https://lore.kernel.org/dmaengine/20260721162822.05CDD1F000E9@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 430b09fa5102e..63f80e2a5b6f4 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
[ ... ]
> @@ -1321,6 +1331,7 @@ static int switchtec_dma_create(struct pci_dev *pdev)
>  	return 0;
>  
>  err_chans_release_exit:

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential NULL
pointer dereference earlier in switchtec_dma_create() in
drivers/dma/switchtec_dma.c.

Can switchtec_dma_create() crash if ioremap() fails? The code maps the PCI BAR
but does not check the return value:

	swdma_dev->bar = ioremap(pci_resource_start(pdev, 0),
				 pci_resource_len(pdev, 0));

If ioremap() returns NULL due to memory exhaustion or an invalid resource, the
subsequent readw() will cause a page fault:

	irq = readw(swdma_dev->bar + SWITCHTEC_REG_CHAN_STS_VEC);


[Severity: High]
This is a pre-existing issue, but I noticed another problem with the IRQ
registration in switchtec_dma_create() in drivers/dma/switchtec_dma.c.

Could the interrupt fire before the channel list is initialized? The IRQ is
requested and enabled before dma_dev->channels is initialized:

	rc = pci_request_irq(pdev, irq, NULL, switchtec_dma_chan_status_isr,
			     swdma_dev, KBUILD_MODNAME);

If the hardware asserts the interrupt immediately after it is requested,
switchtec_dma_chan_status_isr() will execute and locklessly iterate over the
zero-initialized dma_dev->channels list:

	list_for_each_entry(chan, &dma_dev->channels, device_node) {

This leads to iterating over a negative offset pointer, and reading
swdma_chan->dma_chan.dev will fault. Furthermore, even if the list was
initialized, dma_chan.dev is only allocated later in
dma_async_device_register(), so an early interrupt would still dereference a
NULL dev pointer.

> +	switchtec_dma_chans_disable(pdev, swdma_dev);
>  	switchtec_dma_chans_release(pdev, swdma_dev);
>  	switchtec_dma_chans_free(swdma_dev);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902062153.31048-1-logang@deltatee.com?part=6