Re: [PATCH v3 1/1] mmc: 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] mmc: mxs-mmc: mismatched DMA device on dmaengine_prep_slave_sg() error path
- [High] mmc: mxcmmc: NULL pointer dereference in mxcmci_setup_data() on PIO fallback
--

commit 76329da80aaedf4e7a72d175c67b41a9f5e8706a
Author: Frank Li <Frank.Li@nxp.com>

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

This patch replaces direct dma_chan::device::dev access with the proper
dmaengine_get_dma_device() for the consumer API. It ensures the correct
device is used for DMA mapping, preparing for making the DMA engine
provider data structures private.

> diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
> --- a/drivers/mmc/host/mxcmmc.c
> +++ b/drivers/mmc/host/mxcmmc.c
> @@ -290,6 +290,7 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
>  	unsigned int datasize = nob * blksz;
>  	struct scatterlist *sg;
>  	enum dma_transfer_direction slave_dirn;
> +	struct device *dma_dev = dmaengine_get_dma_device(host->dma);

[Severity: High]
Does this code dereference a NULL pointer when host->dma is NULL?

If the driver falls back to PIO mode during probe because requesting the DMA
channel failed, host->dma is set to NULL. This variable initialization appears
to evaluate dmaengine_get_dma_device(host->dma) before the check for PIO mode
(!mxcmci_use_dma(host)) can execute, which could cause a panic.

>  	int i, nents;
>  
>  	host->data = data;

[ ... ]

> diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> --- a/drivers/mmc/host/mxs-mmc.c
> +++ b/drivers/mmc/host/mxs-mmc.c
> @@ -221,7 +221,7 @@ static struct dma_async_tx_descriptor *mxs_mmc_prep_dma(
>  
>  	if (data) {
>  		/* data */
> -		dma_map_sg(mmc_dev(host->mmc), data->sg,
> +		dma_map_sg(dmaengine_get_dma_device(ssp->dmach), data->sg,
>  			   data->sg_len, ssp->dma_dir);
>  		sgl = data->sg;
>  		sg_len = data->sg_len;

[Severity: High]
Will this lead to a mismatched DMA device during the error path in
mxs_mmc_prep_dma()?

The error path in mxs_mmc_prep_dma() still uses the old mmc_dev(host->mmc)
call to unmap the scatterlist:

    if (desc) {
        ...
    } else {
        if (data)
            dma_unmap_sg(mmc_dev(host->mmc), data->sg,
                         data->sg_len, ssp->dma_dir);
    }

Can this leak the IOMMU mapping on the correct device and erroneously unmap
mappings on the wrong device if dmaengine_prep_slave_sg() fails?

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