[PATCH] libxl: qmp: ensure qmp read buffer is NULL terminated

[PATCH] libxl: qmp: ensure qmp read buffer is NULL terminated

[Ian Campbell]

Coverity rightly points out that qmp->buffer may not be NULL
terminated when passed to strncat.

Make the actual buffer a byte bigger than QMP_RECEIVE_BUFFER_SIZE and
always append a NULL byte.

I suspect that in practice we have not yet seen QMP messages
approaching the buffer size (4K).

Compile tested only.

CID: 1055989

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_qmp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 714038b..c45702e 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -67,7 +67,7 @@ struct libxl__qmp_handler {
     /* wait_for_id will be used by the synchronous send function */
     int wait_for_id;
 
-    char buffer[QMP_RECEIVE_BUFFER_SIZE];
+    char buffer[QMP_RECEIVE_BUFFER_SIZE + 1];
     libxl__yajl_ctx *yajl_ctx;
 
     libxl_ctx *ctx;
@@ -457,6 +457,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
             LOGE(ERROR, "Socket read error");
             return rd;
         }
+        qmp->buffer[rd] = '\0';
 
         DEBUG_REPORT_RECEIVED(qmp->buffer, rd);
 
-- 
2.1.4

Re: [PATCH] libxl: qmp: ensure qmp read buffer is NULL terminated

[Ian Campbell]

On Wed, 2016-02-17 at 11:02 +0000, Ian Campbell wrote:
> Coverity rightly points out that qmp->buffer may not be NULL
> terminated when passed to strncat.
> 
> Make the actual buffer a byte bigger than QMP_RECEIVE_BUFFER_SIZE and
> always append a NULL byte.
> 
> I suspect that in practice we have not yet seen QMP messages
> approaching the buffer size (4K).
> 
> Compile tested only.
> 
> CID: 1055989
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

ping.

> ---
>  tools/libxl/libxl_qmp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 714038b..c45702e 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -67,7 +67,7 @@ struct libxl__qmp_handler {
>      /* wait_for_id will be used by the synchronous send function */
>      int wait_for_id;
>  
> -    char buffer[QMP_RECEIVE_BUFFER_SIZE];
> +    char buffer[QMP_RECEIVE_BUFFER_SIZE + 1];
>      libxl__yajl_ctx *yajl_ctx;
>  
>      libxl_ctx *ctx;
> @@ -457,6 +457,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler
> *qmp)
>              LOGE(ERROR, "Socket read error");
>              return rd;
>          }
> +        qmp->buffer[rd] = '\0';
>  
>          DEBUG_REPORT_RECEIVED(qmp->buffer, rd);
>  

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxl: qmp: ensure qmp read buffer is NULL terminated

[Ian Jackson]

Ian Campbell writes ("[PATCH] libxl: qmp: ensure qmp read buffer is NULL terminated"):
> Coverity rightly points out that qmp->buffer may not be NULL
> terminated when passed to strncat.
> 
> Make the actual buffer a byte bigger than QMP_RECEIVE_BUFFER_SIZE and
> always append a NULL byte.
> 
> I suspect that in practice we have not yet seen QMP messages
> approaching the buffer size (4K).
> 
> Compile tested only.
> 
> CID: 1055989
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

and queued

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel