| Message ID | 1455706940-27999-1-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); >
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
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);
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(-)