eal: fix rte_mp_request_sync() memleak on device hotplug

  rte_mp_request_sync() says that the caller is responsible
for freeing one of its parameters afterwards. EAL didn't
do that, causing a memory leak.

Fixes: 244d5130719c ("eal: enable hotplug on multi-process")
Cc: qi.z.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
---
 lib/librte_eal/common/hotplug_mp.c | 2 ++
 1 file changed, 2 insertions(+)
  

On 25-Oct-18 11:46 AM, Darek Stojaczyk wrote:
> rte_mp_request_sync() says that the caller is responsible
> for freeing one of its parameters afterwards. EAL didn't
> do that, causing a memory leak.
> 
> Fixes: 244d5130719c ("eal: enable hotplug on multi-process")
> Cc: qi.z.zhang@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---
>   lib/librte_eal/common/hotplug_mp.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c
> index 84f59d95b..9a6a88546 100644
> --- a/lib/librte_eal/common/hotplug_mp.c
> +++ b/lib/librte_eal/common/hotplug_mp.c
> @@ -355,6 +355,7 @@ int eal_dev_hotplug_request_to_primary(struct eal_dev_mp_req *req)
>   	resp = (struct eal_dev_mp_req *)mp_reply.msgs[0].param;
>   	req->result = resp->result;
>   
> +	free(mp_reply.msgs);
>   	return ret;
>   }
>   
> @@ -397,6 +398,7 @@ int eal_dev_hotplug_request_to_secondary(struct eal_dev_mp_req *req)
>   		}
>   	}
>   
> +	free(mp_reply.msgs);
>   	return 0;
>   }
>   
> 

This is correct but incomplete. There are also numerous error conditions 
which check for number of received responses to be a particular number, 
and if the number don't match, we just exit without freeing memory. 
Those errors need to free the memory as well.
  

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Friday, October 26, 2018 4:22 PM
> To: Stojaczyk, Dariusz <dariusz.stojaczyk@intel.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] eal: fix rte_mp_request_sync() memleak
> on device hotplug
> 
> <SNIP>
>
> This is correct but incomplete. There are also numerous error conditions
> which check for number of received responses to be a particular number,
> and if the number don't match, we just exit without freeing memory.
> Those errors need to free the memory as well.
> 

Yup, thanks. I pushed v2 with one extra free() in the function notifying secondary processes.
The function which notifies the primary process has a similar error check -
 - `if (mp_reply.nb_received == 1)`  - but I figured if there's more than 1 primary process
replying, then the memory leak is your smallest problem.

> --
> Thanks,
> Anatoly
  

> -----Original Message-----
> From: Stojaczyk, Dariusz
> Sent: Monday, October 29, 2018 7:02 AM
> To: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] eal: fix rte_mp_request_sync() memleak on
> device hotplug
> 
> > -----Original Message-----
> > From: Burakov, Anatoly
> > Sent: Friday, October 26, 2018 4:22 PM
> > To: Stojaczyk, Dariusz <dariusz.stojaczyk@intel.com>; dev@dpdk.org
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; stable@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] eal: fix rte_mp_request_sync() memleak
> > on device hotplug
> >
> > <SNIP>
> >
> > This is correct but incomplete. There are also numerous error
> > conditions which check for number of received responses to be a
> > particular number, and if the number don't match, we just exit without
> freeing memory.
> > Those errors need to free the memory as well.
> >
> 
> Yup, thanks. I pushed v2 with one extra free() in the function notifying
> secondary processes.
> The function which notifies the primary process has a similar error check -
>  - `if (mp_reply.nb_received == 1)`  - but I figured if there's more than 1
> primary process replying, then the memory leak is your smallest problem.


Good capture!
Thanks for fix this.

> 
> > --
> > Thanks,
> > Anatoly