diff mbox series

[v6,05/10] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error

Message ID 20260902062153.31048-6-logang@deltatee.com
State New
Headers show
Series dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma | expand

Commit Message

Logan Gunthorpe Sept. 2, 2026, 6:21 a.m. UTC 
chan_status_irq stores an MSI-X vector index, but err_exit freed it
with plain free_irq() instead of pci_free_irq(), which would free the
wrong Linux IRQ. The guard also treated a valid vector index of 0 as
unset, skipping the free entirely in that case and leaving the handler
registered against soon-to-be-freed swdma_dev.

Initialize chan_status_irq to -1 and use the value being non-negative
to signal when to free it with pci_free_irq().

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260717223431.625EE1F000E9@smtp.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

sashiko-bot@kernel.org Sept. 2, 2026, 6:34 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing pci_free_irq_vectors() in the error paths of switchtec_dma_create() violates strict PCI subsystem guidelines.
- [High] Missing NULL check for the result of ioremap() leads to a potential NULL pointer dereference.
--

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

dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error

This commit replaces free_irq() with pci_free_irq() to correctly free MSI-X
vectors. It also changes the default initialization of chan_status_irq to -1
so that index 0 is not skipped during cleanup.

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

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 17bbf85bcb169..430b09fa5102e 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1249,6 +1249,8 @@ static int switchtec_dma_create(struct pci_dev *pdev)
>  	if (!swdma_dev)
>  		return -ENOMEM;
>  
> +	swdma_dev->chan_status_irq = -1;
> +
>  	swdma_dev->bar = ioremap(pci_resource_start(pdev, 0),
>  				 pci_resource_len(pdev, 0));
>  

[Severity: High]
This isn't a bug introduced by this patch, but does ioremap() need to be
checked for failure?

If ioremap() fails and returns NULL, swdma_dev->bar is assigned NULL. A bit
later in switchtec_dma_create(), swdma_dev->bar is dereferenced unconditionally
when reading the IRQ vector:

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

Could this result in a NULL pointer dereference?

[ ... ]

> @@ -1323,8 +1325,8 @@ static int switchtec_dma_create(struct pci_dev *pdev)
>  	switchtec_dma_chans_free(swdma_dev);
>  
>  err_exit:
> -	if (swdma_dev->chan_status_irq)
> -		free_irq(swdma_dev->chan_status_irq, swdma_dev);
> +	if (swdma_dev->chan_status_irq >= 0)
> +		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
>  
>  	iounmap(swdma_dev->bar);
>  	kfree(swdma_dev);

[Severity: High]
This is a pre-existing issue, but does this error path need to free the IRQ
vectors?

In switchtec_dma_create(), if an error occurs after pci_alloc_irq_vectors()
succeeds, control jumps to err_exit and the function returns without calling
pci_free_irq_vectors().

Does this leak IRQ resources on error paths?
diff mbox series

Patch

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 17bbf85bcb16..430b09fa5102 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1249,6 +1249,8 @@  static int switchtec_dma_create(struct pci_dev *pdev)
 	if (!swdma_dev)
 		return -ENOMEM;
 
+	swdma_dev->chan_status_irq = -1;
+
 	swdma_dev->bar = ioremap(pci_resource_start(pdev, 0),
 				 pci_resource_len(pdev, 0));
 
@@ -1323,8 +1325,8 @@  static int switchtec_dma_create(struct pci_dev *pdev)
 	switchtec_dma_chans_free(swdma_dev);
 
 err_exit:
-	if (swdma_dev->chan_status_irq)
-		free_irq(swdma_dev->chan_status_irq, swdma_dev);
+	if (swdma_dev->chan_status_irq >= 0)
+		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
 
 	iounmap(swdma_dev->bar);
 	kfree(swdma_dev);