Re: [PATCH 2/5] dma/idxd: fix memory leak due to free on incorrect pointer

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Apr 08, 2022 at 03:15:01PM +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.
> 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(-)
> 
> diff --git a/drivers/dma/idxd/idxd_pci.c b/drivers/dma/idxd/idxd_pci.c
> index 7036eb938d..fdb1f15750 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);
>  
This is largely my fault, I expect, for being "smart" and allocating the
memory for both arrays from the one allocation. To clarify things, we need
to:
1) update the commit log message explaining why it's the wrong pointer,
i.e. that the two are in the one memory reservation
2) similarly add a comment to the rte_free call noting that it frees the
idx_ring too.

Alternatively, we can also consider adjusting the allocation code so both
arrays are allocated separately, and on free are freed similarly. We would,
however, need to double check that doing so introduces no perf hit.

/Bruce