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

[Kevin Laatz <kevin.laatz@intel.com>]

On 08/04/2022 15:52, Bruce Richardson wrote:
> 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.
>
I'll add explanations/comments where needed in v2, thanks for the feedback.

Kevin