Re: [PATCH 1/3] tools/libxl/libxl_qmp.c: Make sure sun_path is NULL terminated in qmp_open

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Mar 16, 2015 at 10:05:38AM +0000, PRAMOD DEVENDRA wrote:
> From: Pramod Devendra <pramod.devendra@citrix.com>
> 
> Signed-off-by: Pramod Devendra <pramod.devendra@citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_qmp.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index c7324e6..1080162 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -369,10 +369,13 @@ static int qmp_open(libxl__qmp_handler *qmp, const char *qmp_socket_path,
>      ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
>      if (ret) return -1;
>  
> +    if(sizeof (qmp->addr.sun_path) <= strlen(qmp_socket_path))
> +        return -1;
> +

I know this is not your fault, but the function seems to leak qmp_fd on
error path (qmp_fd is not closed). Do you fancy fixing that?

Wei.

>      memset(&qmp->addr, 0, sizeof (qmp->addr));
>      qmp->addr.sun_family = AF_UNIX;
>      strncpy(qmp->addr.sun_path, qmp_socket_path,
> -            sizeof (qmp->addr.sun_path));
> +            sizeof (qmp->addr.sun_path)-1);
>  
>      do {
>          ret = connect(qmp->qmp_fd, (struct sockaddr *) &qmp->addr,
> -- 
> 1.7.10.4