All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Logan Gunthorpe" <logang@deltatee.com>
Cc: vkoul@kernel.org, Frank.Li@kernel.org, linux-pci@vger.kernel.org,
	dmaengine@vger.kernel.org
Subject: Re: [PATCH v4 10/12] dmaengine: ioat: disable relaxed ordering before registering the device
Date: Tue, 28 Jul 2026 17:53:53 +0000	[thread overview]
Message-ID: <20260728175353.BC8AE1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260728171523.112244-11-logang@deltatee.com>

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

Pre-existing issues:
- [High] Use-After-Free (UAF) in shared IRQ handler due to unsafe mixing of manual memory management and devres IRQ registration.
- [High] Global IDA resource leak in the core DMA channel registration error path.
- [Medium] Race condition exposing DMA channels to userspace before their sysfs attributes are fully created.
--

--- Patch [10]: [PATCH v4 10/12] dmaengine: ioat: disable relaxed ordering before registering the device ---
commit f2b93cc9001f8f226c2e33f07702973d69e902de
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: ioat: disable relaxed ordering before registering the device

ioat3_dma_probe() disabled PCIe relaxed ordering after calling
dma_async_device_register(), so if an error occurs and the code jumps
to err_disable_interrupts, the function returns with a dangling registration.
This commit moves the capability read/write ahead of registration to fix
the issue.

Link: https://lore.kernel.org/dmaengine/20260707165906.249F41F000E9@smtp.kernel.org

> diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
> index 737496391109e..a57024c4b066e 100644
> --- a/drivers/dma/ioat/init.c
> +++ b/drivers/dma/ioat/init.c
> @@ -1170,15 +1170,6 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
>  		       ioat_chan->reg_base + IOAT_DCACTRL_OFFSET);
>  	}
>  
> -	err = dma_async_device_register(&ioat_dma->dma_dev);
> -	if (err)
> -		goto err_disable_interrupts;
> -
> -	ioat_kobject_add(ioat_dma, &ioat_ktype);
> -
> -	if (dca)
> -		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
> -
>  	/* disable relaxed ordering */
>  	err = pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &val16);
>  	if (err) {
> @@ -1194,6 +1185,15 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
>  		goto err_disable_interrupts;
>  	}
>  
> +	err = dma_async_device_register(&ioat_dma->dma_dev);

[Severity: High]
This is a pre-existing issue, but does dma_async_device_register() leak the
global dma_ida resource when it fails?

If dma_async_device_register() fails after allocating a global ID and
populating the channel list, the error path in drivers/dma/dmaengine.c handles
the cleanup:

	if (!device->chancnt) {
		ida_free(&dma_ida, device->dev_id);
		return rc;
	}

Because it skips ida_free() when chancnt is greater than zero, does this
permanently leak an ID from the global dma_ida pool every time registration
fails with populated channels?

> +	if (err)
> +		goto err_disable_interrupts;

[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free risk
on this error path?

If ioat3_dma_probe() fails and the code takes the err_disable_interrupts
path, ioat_pci_probe() will manually free the primary device data structure.
However, because ioat_dma_setup_interrupts() registers a shared interrupt
using devm_request_irq(), the interrupt cleanup is deferred until after the
probe function completely returns.

Could a shared interrupt firing during this brief window invoke the handler,
causing ioat_dma_do_interrupt() to blindly dereference the already freed data
pointer?

> +
> +	ioat_kobject_add(ioat_dma, &ioat_ktype);

[Severity: Medium]
This is a pre-existing issue, but does this sequence expose DMA channels to
userspace before their sysfs attributes are fully created?

Because dma_async_device_register() completes core registration and sends
add uevents before ioat_kobject_add() appends the specific quickdata
sysfs attributes, can a userspace process reacting to the uevent attempt to
read device properties in the window before they are created?

> +
> +	if (dca)
> +		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
> +
>  	if (ioat_dma->cap & IOAT_CAP_DPS)
>  		writeb(ioat_pending_level + 1,
>  		       ioat_dma->reg_base + IOAT_PREFETCH_LIMIT_OFFSET);

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

  reply	other threads:[~2026-07-28 17:53 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 17:15 [PATCH v4 00/12] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
2026-07-28 17:15 ` [PATCH v4 01/12] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc() Logan Gunthorpe
2026-07-28 17:34   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 02/12] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources Logan Gunthorpe
2026-07-28 17:40   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 03/12] dmaengine: switchtec-dma: always clear DMA base registers on chan_stop() Logan Gunthorpe
2026-07-28 17:34   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 04/12] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error Logan Gunthorpe
2026-07-28 17:35   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 05/12] dmaengine: switchtec-dma: fix channel leak on registration failure Logan Gunthorpe
2026-07-28 17:38   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 06/12] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void Logan Gunthorpe
2026-07-28 17:44   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 07/12] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error Logan Gunthorpe
2026-07-28 17:39   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 08/12] dmaengine: switchtec-dma: disable channels before freeing on registration failure Logan Gunthorpe
2026-07-28 17:39   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 09/12] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove() Logan Gunthorpe
2026-07-28 17:48   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 10/12] dmaengine: ioat: disable relaxed ordering before registering the device Logan Gunthorpe
2026-07-28 17:53   ` sashiko-bot [this message]
2026-07-28 17:15 ` [PATCH v4 11/12] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show() Logan Gunthorpe
2026-07-28 17:48   ` sashiko-bot
2026-07-28 17:15 ` [PATCH v4 12/12] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr() Logan Gunthorpe
2026-07-28 17:50   ` 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=20260728175353.BC8AE1F00A3A@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.