Re: [dpdk-dev] [PATCH] vfio: free mp_reply msgs in failure cases

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 16-Aug-19 1:13 PM, Jim Harris wrote:
> The code checks both rte_mp_request_sync() return
> code and that the number of messages in the reply
> equals 1.  If rte_mp_request_sync() succeeds but
> there was more than one message, those messages
> would get leaked.
> 
> Found via code review by Anatoly Burakov of patches
> that used the vhost code as a template for using
> rte_mp_request_sync().
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> ---
>   lib/librte_eal/linux/eal/eal_vfio.c |   16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/librte_eal/linux/eal/eal_vfio.c b/lib/librte_eal/linux/eal/eal_vfio.c
> index 501c74f23..d9541b122 100644
> --- a/lib/librte_eal/linux/eal/eal_vfio.c
> +++ b/lib/librte_eal/linux/eal/eal_vfio.c
> @@ -264,7 +264,7 @@ vfio_open_group_fd(int iommu_group_num)
>   	int vfio_group_fd;
>   	char filename[PATH_MAX];
>   	struct rte_mp_msg mp_req, *mp_rep;
> -	struct rte_mp_reply mp_reply;
> +	struct rte_mp_reply mp_reply = {0};
>   	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
>   	struct vfio_mp_param *p = (struct vfio_mp_param *)mp_req.param;
>   
> @@ -320,9 +320,9 @@ vfio_open_group_fd(int iommu_group_num)
>   			RTE_LOG(ERR, EAL, "  bad VFIO group fd\n");
>   			vfio_group_fd = 0;
>   		}
> -		free(mp_reply.msgs);
>   	}
>   
> +	free(mp_reply.msgs);

That's not quite correct. This fixes the problem of missing free() when 
nb_received mismatches, but this /adds/ a problem of doing an 
unnecessary free() when rte_mp_request_sync() returns -1. Same for other 
places, i believe.

-- 
Thanks,
Anatoly