Re: [PATCH v3 22/23] dmaengine: sdxi: MSI/MSI-X vector allocation and mapping

[sashiko-bot@kernel.org]

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

New issues:
- [High] Failure to check the return value of dma_set_mask_and_coherent().

Pre-existing issues:
- [Critical] Teardown ordering risks Use-After-Free: Software tracking structures and DMA memory are destroyed before the hardware is stopped.
--

commit 31c552b5f89a074f21648e62e100ec4d29388cf3
Author: Nathan Lynch <nathan.lynch@amd.com>

dmaengine: sdxi: MSI/MSI-X vector allocation and mapping

During PCI probe, allocate a vector per context supported by the
function as reported by the capability register, plus one for the
error log interrupt. Introduce wrapper functions for vector
allocation, freeing, and translation to IRQ numbers.

> diff --git a/drivers/dma/sdxi/device.c b/drivers/dma/sdxi/device.c
> --- a/drivers/dma/sdxi/device.c
> +++ b/drivers/dma/sdxi/device.c
[ ... ]
> @@ -355,6 +357,8 @@ void sdxi_unregister(struct device *dev)
>  		sdxi_cxt_exit(cxt);
>  	xa_destroy(&sdxi->client_cxts);
>  
> +	ida_destroy(&sdxi->vectors);
> +
>  	sdxi_dev_stop(sdxi);
>  }

[Severity: Critical]
This isn't a bug introduced by this patch, but does this teardown ordering
risk a use-after-free?

In sdxi_unregister(), the driver destroys all client contexts and their
DMA descriptor rings before sdxi_dev_stop() is called to quiesce the
hardware.

Because the hardware is still active, could it write to the freed status
rings or generate interrupts, causing sdxi_dma_cxt_irq() to dereference
freed memory?

Additionally, since the DMA engine framework is unregistered after
sdxi_unregister() completes during devres cleanup, could a client freeing
a channel in that phase cause sdxi_dma_free_chan_resources() to double-free
the already destroyed context?

> diff --git a/drivers/dma/sdxi/pci.c b/drivers/dma/sdxi/pci.c
> --- a/drivers/dma/sdxi/pci.c
> +++ b/drivers/dma/sdxi/pci.c
> @@ -29,7 +31,8 @@ static int sdxi_pci_init(struct sdxi_dev *sdxi)
>  {
>  	struct pci_dev *pdev = sdxi_to_pci_dev(sdxi);
>  	struct device *dev = &pdev->dev;
> -	int ret;
> +	unsigned int cap1_max_cxt;
> +	int vecs, ret;
>  
>  	ret = pcim_enable_device(pdev);
>  	if (ret)

[Severity: High]
Does this function need to check the return value of
dma_set_mask_and_coherent()?

Looking at sdxi_pci_init(), it unconditionally configures a 64-bit DMA mask:

    dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));

If the platform or bus rejects the 64-bit mask, could the driver proceed
to allocate and map DMA buffers under the false assumption that 64-bit
addressing is supported, resulting in silently truncated physical addresses
or IOMMU faults?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-sdxi-base-v3-0-4d38ca2bdffe@amd.com?part=22