[PATCH] dmaengine: fsl-dpaa2-qdma: fix ppriv memory leaks

[PATCH] dmaengine: fsl-dpaa2-qdma: fix ppriv memory leaks

[Guangshuo Li]

dpaa2_qdma_setup() allocates priv->ppriv separately from priv, but the
allocation is not released on all teardown paths.

If dpdmai_get_rx_queue() or dpdmai_get_tx_queue() fails after
priv->ppriv has been allocated, dpaa2_qdma_setup() returns through the
error path without freeing it.

The normal remove path has the same issue. dpaa2_qdma_remove() frees
priv without first releasing priv->ppriv, losing the only reference to
the separately allocated array.

Free priv->ppriv on setup failures that occur after its allocation and
during normal driver removal before freeing priv.

This issue was found by manual code inspection.

Fixes: 7fdf9b05c73b ("dmaengine: fsl-dpaa2-qdma: Add NXP dpaa2 qDMA controller driver for Layerscape SoCs")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
index bf771251264d..0f0fe0291a06 100644
--- a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
+++ b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
@@ -365,7 +365,7 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
 					  i, 0, &priv->rx_queue_attr[i]);
 		if (err) {
 			dev_err(dev, "dpdmai_get_rx_queue() failed\n");
-			goto exit;
+			goto err_free_ppriv;
 		}
 		ppriv->rsp_fqid = priv->rx_queue_attr[i].fqid;
 
@@ -373,7 +373,7 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
 					  i, 0, &priv->tx_queue_attr[i]);
 		if (err) {
 			dev_err(dev, "dpdmai_get_tx_queue() failed\n");
-			goto exit;
+			goto err_free_ppriv;
 		}
 		ppriv->req_fqid = priv->tx_queue_attr[i].fqid;
 		ppriv->prio = DPAA2_QDMA_DEFAULT_PRIORITY;
@@ -382,6 +382,9 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
 	}
 
 	return 0;
+
+err_free_ppriv:
+	kfree(priv->ppriv);
 exit:
 	dpdmai_close(priv->mc_io, 0, ls_dev->mc_handle);
 	return err;
@@ -787,6 +790,7 @@ static void dpaa2_qdma_remove(struct fsl_mc_device *ls_dev)
 	dpaa2_dpdmai_free_channels(dpaa2_qdma);
 
 	dma_async_device_unregister(&dpaa2_qdma->dma_dev);
+	kfree(priv->ppriv);
 	kfree(priv);
 	kfree(dpaa2_qdma);
 }
-- 
2.43.0

Re: [PATCH] dmaengine: fsl-dpaa2-qdma: fix ppriv memory leaks

[sashiko-bot]

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

Pre-existing issues:
- [High] Double free of DMA pools (`fd_pool`, `fl_pool`, `sdd_pool`) during driver removal if channels were previously used.
- [High] Incorrect teardown sequence in `dpaa2_qdma_remove()` destroys hardware state and channel resources before unregistering the DMA device.
- [High] Missing `tasklet_kill()` leads to Use-After-Free on the tasklet structures during driver removal.
- [High] Race condition in `dpaa2_qdma_remove()` leading to Use-After-Free in concurrent DPIO notifications.
--

commit 7bbc2b15c0a3c76af93c56c1e6c4036a65094cf2
Author: Guangshuo Li <lgs201920130244@gmail.com>

dmaengine: fsl-dpaa2-qdma: fix ppriv memory leaks

dpaa2_qdma_setup() allocates priv->ppriv separately from priv, but the
allocation is not released on all teardown paths. This commit frees
priv->ppriv on setup failures and during driver removal.

> diff --git a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
> index bf771251264d9..0f0fe0291a06f 100644
> --- a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
> +++ b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
[ ... ]
> @@ -787,6 +790,7 @@ static void dpaa2_qdma_remove(struct fsl_mc_device *ls_dev)

[Severity: High]
This is a pre-existing issue, but is there a risk of a use-after-free
in the DPIO hardware notification callback during unbind?

Earlier in dpaa2_qdma_remove(), the DPIO store memory is freed:

    dpaa2_dpmai_store_free(priv);
    dpaa2_dpdmai_dpio_free(priv);

If a hardware notification triggers dpaa2_qdma_fqdan_cb() concurrently
between these two calls, it will heavily access the freed ppriv->store
memory. Should the DPIO service be deregistered before the memory it
depends on is destroyed?

>  	dpaa2_dpdmai_free_channels(dpaa2_qdma);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for
dpaa2_dpdmai_free_channels() to cause a double free of the DMA pools here?

When a client releases a DMA channel, dpaa2_qdma_free_chan_resources()
destroys the pools but leaves the pointers intact:

drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c:dpaa2_qdma_free_chan_resources() {
    ...
    dma_pool_destroy(dpaa2_chan->fd_pool);
    dma_pool_destroy(dpaa2_chan->fl_pool);
    dma_pool_destroy(dpaa2_chan->sdd_pool);
    ...
}

If the driver is later unbound, dpaa2_qdma_remove() unconditionally calls
dpaa2_dpdmai_free_channels(), which iterates over the channels and calls
dma_pool_destroy() on these same pointers again. Could this lead to a
panic or memory corruption in the SLUB allocator?

>  
>  	dma_async_device_unregister(&dpaa2_qdma->dma_dev);

[Severity: High]
This is a pre-existing issue, but does this teardown sequence destroy
hardware state and channel resources while the device is still exposed to
clients?

Because dma_async_device_unregister() is called after
dpaa2_dpdmai_free_channels(), the DMA device is still registered with the
DMA core during hardware teardown. Couldn't concurrent clients still submit
operations or request channels, attempting to access already-destroyed pools
or freed portals?

> +	kfree(priv->ppriv);
>  	kfree(priv);
>  	kfree(dpaa2_qdma);

[Severity: High]
This isn't a bug introduced by this patch, but does freeing dpaa2_qdma
here cause a use-after-free for pending tasklets?

The driver initializes a tasklet for each channel during setup:

drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c:dpaa2_dpdmai_init_channels() {
    ...
    vchan_init(&dpaa2_chan->vchan, &dpaa2_qdma->dma_dev);
    ...
}

During removal, kfree(dpaa2_qdma) frees the structure containing these
channels and their embedded tasklets, but tasklet_kill() is never called.
If a tasklet is scheduled when the module unbinds, could this lead to a
use-after-free when the pending tasklet executes?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913045001.1126563-1-lgs201920130244@gmail.com?part=1

Re: [PATCH] dmaengine: fsl-dpaa2-qdma: fix ppriv memory leaks

[Frank Li]

On Sun, Sep 13, 2026 at 12:50:01PM +0800, Guangshuo Li wrote:
> dpaa2_qdma_setup() allocates priv->ppriv separately from priv, but the
> allocation is not released on all teardown paths.
>
> If dpdmai_get_rx_queue() or dpdmai_get_tx_queue() fails after
> priv->ppriv has been allocated, dpaa2_qdma_setup() returns through the
> error path without freeing it.
>
> The normal remove path has the same issue. dpaa2_qdma_remove() frees
> priv without first releasing priv->ppriv, losing the only reference to
> the separately allocated array.
>
> Free priv->ppriv on setup failures that occur after its allocation and
> during normal driver removal before freeing priv.
>
> This issue was found by manual code inspection.
>
> Fixes: 7fdf9b05c73b ("dmaengine: fsl-dpaa2-qdma: Add NXP dpaa2 qDMA controller driver for Layerscape SoCs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---

qdma_setup() is called by probe() only use devm_ allocate function

Frank

>  drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
> index bf771251264d..0f0fe0291a06 100644
> --- a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
> +++ b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
> @@ -365,7 +365,7 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
>  					  i, 0, &priv->rx_queue_attr[i]);
>  		if (err) {
>  			dev_err(dev, "dpdmai_get_rx_queue() failed\n");
> -			goto exit;
> +			goto err_free_ppriv;
>  		}
>  		ppriv->rsp_fqid = priv->rx_queue_attr[i].fqid;
>
> @@ -373,7 +373,7 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
>  					  i, 0, &priv->tx_queue_attr[i]);
>  		if (err) {
>  			dev_err(dev, "dpdmai_get_tx_queue() failed\n");
> -			goto exit;
> +			goto err_free_ppriv;
>  		}
>  		ppriv->req_fqid = priv->tx_queue_attr[i].fqid;
>  		ppriv->prio = DPAA2_QDMA_DEFAULT_PRIORITY;
> @@ -382,6 +382,9 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
>  	}
>
>  	return 0;
> +
> +err_free_ppriv:
> +	kfree(priv->ppriv);
>  exit:
>  	dpdmai_close(priv->mc_io, 0, ls_dev->mc_handle);
>  	return err;
> @@ -787,6 +790,7 @@ static void dpaa2_qdma_remove(struct fsl_mc_device *ls_dev)
>  	dpaa2_dpdmai_free_channels(dpaa2_qdma);
>
>  	dma_async_device_unregister(&dpaa2_qdma->dma_dev);
> +	kfree(priv->ppriv);
>  	kfree(priv);
>  	kfree(dpaa2_qdma);
>  }
> --
> 2.43.0
>

Re: [PATCH] dmaengine: fsl-dpaa2-qdma: fix ppriv memory leaks

[Guangshuo Li]

Hi Frank,

Thanks for the review.

On Mon, 14 Sept 2026 at 23:12, Frank Li <Frank.li@oss.nxp.com> wrote:
>
> On Sun, Sep 13, 2026 at 12:50:01PM +0800, Guangshuo Li wrote:
> > dpaa2_qdma_setup() allocates priv->ppriv separately from priv, but the
> > allocation is not released on all teardown paths.
> >
> > If dpdmai_get_rx_queue() or dpdmai_get_tx_queue() fails after
> > priv->ppriv has been allocated, dpaa2_qdma_setup() returns through the
> > error path without freeing it.
> >
> > The normal remove path has the same issue. dpaa2_qdma_remove() frees
> > priv without first releasing priv->ppriv, losing the only reference to
> > the separately allocated array.
> >
> > Free priv->ppriv on setup failures that occur after its allocation and
> > during normal driver removal before freeing priv.
> >
> > This issue was found by manual code inspection.
> >
> > Fixes: 7fdf9b05c73b ("dmaengine: fsl-dpaa2-qdma: Add NXP dpaa2 qDMA controller driver for Layerscape SoCs")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
>
> qdma_setup() is called by probe() only use devm_ allocate function
>
> Frank
>
> >  drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
> > index bf771251264d..0f0fe0291a06 100644
> > --- a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
> > +++ b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c
> > @@ -365,7 +365,7 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
> >                                         i, 0, &priv->rx_queue_attr[i]);
> >               if (err) {
> >                       dev_err(dev, "dpdmai_get_rx_queue() failed\n");
> > -                     goto exit;
> > +                     goto err_free_ppriv;
> >               }
> >               ppriv->rsp_fqid = priv->rx_queue_attr[i].fqid;
> >
> > @@ -373,7 +373,7 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
> >                                         i, 0, &priv->tx_queue_attr[i]);
> >               if (err) {
> >                       dev_err(dev, "dpdmai_get_tx_queue() failed\n");
> > -                     goto exit;
> > +                     goto err_free_ppriv;
> >               }
> >               ppriv->req_fqid = priv->tx_queue_attr[i].fqid;
> >               ppriv->prio = DPAA2_QDMA_DEFAULT_PRIORITY;
> > @@ -382,6 +382,9 @@ static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev)
> >       }
> >
> >       return 0;
> > +
> > +err_free_ppriv:
> > +     kfree(priv->ppriv);
> >  exit:
> >       dpdmai_close(priv->mc_io, 0, ls_dev->mc_handle);
> >       return err;
> > @@ -787,6 +790,7 @@ static void dpaa2_qdma_remove(struct fsl_mc_device *ls_dev)
> >       dpaa2_dpdmai_free_channels(dpaa2_qdma);
> >
> >       dma_async_device_unregister(&dpaa2_qdma->dma_dev);
> > +     kfree(priv->ppriv);
> >       kfree(priv);
> >       kfree(dpaa2_qdma);
> >  }
> > --
> > 2.43.0
> >

Agreed. I'll switch priv->ppriv to devm_kcalloc() and send a v2.

Thanks,
Guangshuo