Linux PCI subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Logan Gunthorpe" <logang@deltatee.com>
Cc: dmaengine@vger.kernel.org, Frank.Li@kernel.org,
	linux-pci@vger.kernel.org, vkoul@kernel.org
Subject: Re: [PATCH v5 01/12] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc()
Date: Mon, 31 Aug 2026 21:28:04 +0000	[thread overview]
Message-ID: <20260831212804.F07EB1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260831191403.207631-2-logang@deltatee.com>

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

Pre-existing issues:
- [High] switchtec_dma_prep_desc() acquires and returns with a spinlock held, violating the dmaengine API and causing inevitable deadlocks.
--

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

dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc()

This commit prevents a double-free in switchtec_dma_free_desc() by setting
hw_sq, hw_cq, and desc_ring entries to NULL after freeing them. This fixes
an issue during channel resource allocation failures where unwind paths
could attempt to free already-released pointers.

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index c133535d37656..a10818efba4e5 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -886,14 +886,18 @@ static void switchtec_dma_free_desc(struct switchtec_dma_chan *swdma_chan)
>  	if (swdma_chan->hw_sq)
>  		dma_free_coherent(swdma_dev->dma_dev.dev, size,
>  				  swdma_chan->hw_sq, swdma_chan->dma_addr_sq);
> +	swdma_chan->hw_sq = NULL;
>  
>  	size = SWITCHTEC_DMA_CQ_SIZE * sizeof(*swdma_chan->hw_cq);
[ ... ]

While reviewing this file, I noticed a separate issue in
switchtec_dma_prep_desc():

drivers/dma/switchtec_dma.c:switchtec_dma_prep_desc() {
	spin_lock_bh(&swdma_chan->submit_lock);

	if (!swdma_chan->ring_active)
		goto err_unlock;
	...
	/* return with the lock held, it will be released in tx_submit */

	return &desc->txd;
}

[Severity: High]
This isn't a bug introduced by this patch, but does returning with the
submit_lock held violate the dmaengine API contract and cause deadlocks?

The dmaengine API allows clients to batch multiple descriptor preparations
before submitting them. If a client calls the prep function consecutively,
would the second call attempt to re-acquire the same non-recursive spinlock
and deadlock the CPU?

Additionally, the dmaengine API allows aborting and dropping descriptors
on error before submission. If a prepared descriptor is discarded on a
client error path, would the lock remain permanently held?

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

  reply	other threads:[~2026-08-31 21:28 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 19:13 [PATCH v5 00/12] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
2026-08-31 19:13 ` [PATCH v5 01/12] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc() Logan Gunthorpe
2026-08-31 21:28   ` sashiko-bot [this message]
2026-08-31 19:13 ` [PATCH v5 02/12] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources Logan Gunthorpe
2026-08-31 21:38   ` sashiko-bot
2026-08-31 19:13 ` [PATCH v5 03/12] dmaengine: switchtec-dma: always clear DMA base registers on chan_stop() Logan Gunthorpe
2026-08-31 21:51   ` sashiko-bot
2026-08-31 22:43     ` Logan Gunthorpe
2026-08-31 19:13 ` [PATCH v5 04/12] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error Logan Gunthorpe
2026-08-31 22:04   ` sashiko-bot
2026-08-31 19:13 ` [PATCH v5 05/12] dmaengine: switchtec-dma: fix channel leak on registration failure Logan Gunthorpe
2026-08-31 22:18   ` sashiko-bot
2026-08-31 19:13 ` [PATCH v5 06/12] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void Logan Gunthorpe
2026-08-31 22:24   ` sashiko-bot
2026-08-31 19:13 ` [PATCH v5 07/12] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error Logan Gunthorpe
2026-08-31 22:35   ` sashiko-bot
2026-08-31 19:13 ` [PATCH v5 08/12] dmaengine: switchtec-dma: disable channels before freeing on registration failure Logan Gunthorpe
2026-08-31 22:50   ` sashiko-bot
2026-08-31 19:13 ` [PATCH v5 09/12] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove() Logan Gunthorpe
2026-08-31 23:04   ` sashiko-bot
2026-08-31 19:14 ` [PATCH v5 10/12] dmaengine: ioat: disable relaxed ordering before registering the device Logan Gunthorpe
2026-08-31 23:13   ` sashiko-bot
2026-08-31 19:14 ` [PATCH v5 11/12] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show() Logan Gunthorpe
2026-08-31 23:19   ` sashiko-bot
2026-08-31 19:14 ` [PATCH v5 12/12] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr() Logan Gunthorpe
2026-08-31 23:30   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260831212804.F07EB1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=logang@deltatee.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox