Re: [PATCH 1/1] mtd: 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] mtd: NULL pointer dereference in am654_hbmc_dma_read
- [Medium] mtd: Asymmetric DMA mapping and unmapping in cadence-nand-controller
--

commit c06f81e901974be1dc19c7a1842c1db726a5fe88
Author: Frank Li <Frank.Li@nxp.com>

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

This commit replaces direct dma_chan device struct access with the proper
dmaengine_get_dma_device() consumer API across multiple MTD drivers. It
returns the correct device for each channel when DMA engines support
per-channel IOMMU mappings, and prepares for making provider structures
private.

> diff --git a/drivers/mtd/hyperbus/hbmc-am654.c b/drivers/mtd/hyperbus/hbmc-am654.c
> index 9d31464046b22..8a14e908d0b53 100644
> --- a/drivers/mtd/hyperbus/hbmc-am654.c
> +++ b/drivers/mtd/hyperbus/hbmc-am654.c
> @@ -78,14 +78,15 @@ static int am654_hbmc_dma_read(struct am654_hbmc_device_priv *priv, void *to,
>  	struct dma_chan *rx_chan = priv->rx_chan;
>  	struct dma_async_tx_descriptor *tx;
>  	dma_addr_t dma_dst, dma_src;
> +	struct device *rx_dev = dmaengine_get_dma_device(rx_chan);

[Severity: High]
If rx_chan is NULL, will this cause an immediate NULL pointer dereference?

Based on the driver implementation, rx_chan can be NULL if the DMA fallback
to PIO is taken. The new dmaengine_get_dma_device() function will dereference
the channel pointer unconditionally:

include/linux/dmaengine.h:dmaengine_get_dma_device() {
    if (chan->dev->chan_dma_dev)
...

Should rx_dev be initialized after the if (!priv->rx_chan) check below?

>  	dma_cookie_t cookie;
>  	int ret;
>  
>  	if (!priv->rx_chan || !virt_addr_valid(to) || object_is_on_stack(to))
>  		return -EINVAL;
>  

[ ... ]

> diff --git a/drivers/mtd/nand/raw/cadence-nand-controller.c b/drivers/mtd/nand/raw/cadence-nand-controller.c
> index 2085632631c02..4f5e6d2b128c7 100644
> --- a/drivers/mtd/nand/raw/cadence-nand-controller.c
> +++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
> @@ -3229,7 +3229,7 @@ static void cadence_nand_remove(struct cdns_nand_ctrl *cdns_ctrl)
>  {
>  	cadence_nand_chips_cleanup(cdns_ctrl);
>  	if (cdns_ctrl->dmac)
> -		dma_unmap_resource(cdns_ctrl->dmac->device->dev,
> +		dma_unmap_resource(dmaengine_get_dma_device(cdns_ctrl->dmac),
>  				   cdns_ctrl->io.iova_dma, cdns_ctrl->io.size,
>  				   DMA_BIDIRECTIONAL, 0);

[Severity: Medium]
Does this introduce an asymmetric DMA mapping and unmapping mismatch?

The original mapping in cadence_nand_init() still uses the old pattern:

cdns_ctrl->io.iova_dma = dma_map_resource(dma_dev->dev, cdns_ctrl->io.dma,
    cdns_ctrl->io.size, DMA_BIDIRECTIONAL, 0);

If a provider uses per-channel devices, these pointers could differ,
potentially causing resource leaks or kernel warnings on device unbind.
Should cadence_nand_init() also be updated to use dmaengine_get_dma_device()
to maintain symmetry?

[ ... ]

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