[v2,2/3] dma/idxd: fix memory leak due to free on incorrect pointer

Message ID 20220703122243.929642-3-kevin.laatz@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Fix IDXD PCI device close |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Kevin Laatz July 3, 2022, 12:22 p.m. UTC
  During PCI device close, any allocated memory needs to be free'd.
Currently, one of the free's is being called on an incorrect idxd_dmadev
struct member, namely 'batch_idx_ring', causing a memleak from the
pointer that should have been free'd.
This patch fixes this memleak by calling free on the correct pointer.

Fixes: 9449330a8458 ("dma/idxd: create dmadev instances on PCI probe")
Cc: stable@dpdk.org
Cc: bruce.richardson@intel.com

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 drivers/dma/idxd/idxd_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Bruce Richardson July 4, 2022, 1:23 p.m. UTC | #1
On Sun, Jul 03, 2022 at 01:22:42PM +0100, Kevin Laatz wrote:
> During PCI device close, any allocated memory needs to be free'd.
> Currently, one of the free's is being called on an incorrect idxd_dmadev
> struct member, namely 'batch_idx_ring', causing a memleak from the
> pointer that should have been free'd.

I think you need to explain that the two rings are beside each other in
memory and we need to free using the pointer to the start of the block,
rather than the pointer to the middle of it.

> This patch fixes this memleak by calling free on the correct pointer.
> 
> Fixes: 9449330a8458 ("dma/idxd: create dmadev instances on PCI probe")
> Cc: stable@dpdk.org
> Cc: bruce.richardson@intel.com
> 
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>

With more explanation in the commit log

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  drivers/dma/idxd/idxd_pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/idxd/idxd_pci.c b/drivers/dma/idxd/idxd_pci.c
> index 918981f2ea..9349c56b3f 100644
> --- a/drivers/dma/idxd/idxd_pci.c
> +++ b/drivers/dma/idxd/idxd_pci.c
> @@ -130,7 +130,7 @@ idxd_pci_dev_close(struct rte_dma_dev *dev)
>  
>  	/* free device memory */
>  	IDXD_PMD_DEBUG("Freeing device driver memory");
> -	rte_free(idxd->batch_idx_ring);
> +	rte_free(idxd->batch_comp_ring);
>  	rte_free(idxd->desc_ring);
>  
>  	/* if this is the last WQ on the device, disable the device and free
> -- 
> 2.31.1
>
  
Bruce Richardson July 4, 2022, 1:25 p.m. UTC | #2
On Mon, Jul 04, 2022 at 02:23:38PM +0100, Bruce Richardson wrote:
> On Sun, Jul 03, 2022 at 01:22:42PM +0100, Kevin Laatz wrote:
> > During PCI device close, any allocated memory needs to be free'd.
> > Currently, one of the free's is being called on an incorrect idxd_dmadev
> > struct member, namely 'batch_idx_ring', causing a memleak from the
> > pointer that should have been free'd.
> 
> I think you need to explain that the two rings are beside each other in
> memory and we need to free using the pointer to the start of the block,
> rather than the pointer to the middle of it.
> 
> > This patch fixes this memleak by calling free on the correct pointer.
> > 
> > Fixes: 9449330a8458 ("dma/idxd: create dmadev instances on PCI probe")
> > Cc: stable@dpdk.org
> > Cc: bruce.richardson@intel.com
> > 
> > Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> 
> With more explanation in the commit log
> 

Correction (obviously!)
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

Patch

diff --git a/drivers/dma/idxd/idxd_pci.c b/drivers/dma/idxd/idxd_pci.c
index 918981f2ea..9349c56b3f 100644
--- a/drivers/dma/idxd/idxd_pci.c
+++ b/drivers/dma/idxd/idxd_pci.c
@@ -130,7 +130,7 @@  idxd_pci_dev_close(struct rte_dma_dev *dev)
 
 	/* free device memory */
 	IDXD_PMD_DEBUG("Freeing device driver memory");
-	rte_free(idxd->batch_idx_ring);
+	rte_free(idxd->batch_comp_ring);
 	rte_free(idxd->desc_ring);
 
 	/* if this is the last WQ on the device, disable the device and free