Re: [PATCH 2/2] spi: use dmaengine_get_dma_device() instead of chan->device->dev

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] spi: bcm2835: NULL pointer dereference in bcm2835_spi_setup_dma()
- [High] spi: fsl-dspi: NULL pointer dereference in dspi_release_dma()
--

commit 94e56ea57266b861e4fa8e6aaf7104da2aa355f5
Author: Frank Li <Frank.Li@nxp.com>

spi: use dmaengine_get_dma_device() instead of chan->device->dev

This commit replaces direct access to dma_chan::device::dev with the proper
dmaengine_get_dma_device() consumer API across various SPI drivers. This
prepares the codebase for making the DMA engine provider data structures
private.

Link: https://lore.kernel.org/imx/67966b47-22cb-4b07-80c7-2044f557dbcb@app.fastmail.com/

> diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
> index 23606967f7d1f..faec657db2ffe 100644
> --- a/drivers/spi/spi-bcm2835.c
> +++ b/drivers/spi/spi-bcm2835.c
[ ... ]
> @@ -1174,16 +1177,15 @@ static int bcm2835_spi_setup_dma(struct spi_controller *ctlr,
>  				 struct bcm2835_spi *bs,
>  				 struct bcm2835_spidev *target)
>  {
> +	struct device *rx_dev = dmaengine_get_dma_device(ctlr->dma_rx);
>  	int ret;
>  
>  	if (!ctlr->dma_rx)
>  		return 0;

[Severity: High]
Does this code dereference ctlr->dma_rx before checking if it is NULL?

If DMA configuration fails or is unavailable during controller initialization
in bcm2835_dma_init(), the driver gracefully falls back to interrupt mode
and sets ctlr->dma_rx to NULL. When a new SPI device is later set up via
bcm2835_spi_setup(), it unconditionally calls bcm2835_spi_setup_dma().

Because the macro dmaengine_get_dma_device() dereferences the passed
channel pointer, doing so before the if (!ctlr->dma_rx) guard could trigger
a kernel panic if DMA is disabled.

[ ... ]
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index 23e6bc5f202e2..25af5e6777230 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
[ ... ]
> @@ -766,21 +769,21 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
>  static void dspi_release_dma(struct fsl_dspi *dspi)
>  {
>  	struct fsl_dspi_dma *dma = dspi->dma;
> +	struct device *tx_dev = dmaengine_get_dma_device(dma->chan_tx);
> +	struct device *rx_dev = dmaengine_get_dma_device(dma->chan_rx);
>  
>  	if (!dma)
>  		return;

[Severity: High]
Can this lead to a NULL pointer dereference when accessing dma->chan_tx and
dma->chan_rx?

If the driver is configured in a mode that doesn't use DMA,
dspi_request_dma() is bypassed, and dspi->dma remains NULL. Similarly, if
dspi_request_dma() fails, it bails out before assigning dspi->dma.

During teardown or probe error rollback, dspi_release_dma() is called. If
dma is NULL, fetching dma->chan_tx to pass to dmaengine_get_dma_device()
before the if (!dma) safety guard is reached would cause a crash.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929155445.3311133-1-Frank.Li@oss.nxp.com?part=2