> -----Original Message----- > From: Jason Wang [mailto:jasow...@redhat.com] > Sent: Thursday, July 11, 2019 4:18 AM > To: Zhang, Chen <chen.zh...@intel.com>; Li Zhijian <lizhij...@cn.fujitsu.com>; > Peter Maydell <peter.mayd...@linaro.org>; qemu-dev <qemu- > de...@nongnu.org> > Cc: Zhang Chen <zhangc...@gmail.com> > Subject: Re: [Qemu-devel] [PATCH V2] net/colo-compare.c: Fix memory leak > and code style issue. > > > On 2019/7/10 下午3:50, Zhang, Chen wrote: > > > >> -----Original Message----- > >> From: Jason Wang [mailto:jasow...@redhat.com] > >> Sent: Tuesday, July 9, 2019 10:48 PM > >> To: Zhang, Chen <chen.zh...@intel.com>; Li Zhijian > >> <lizhij...@cn.fujitsu.com>; Peter Maydell <peter.mayd...@linaro.org>; > >> qemu-dev <qemu- de...@nongnu.org> > >> Cc: Zhang Chen <zhangc...@gmail.com> > >> Subject: Re: [Qemu-devel] [PATCH V2] net/colo-compare.c: Fix memory > >> leak and code style issue. > >> > >> > >> On 2019/7/4 下午4:36, Zhang Chen wrote: > >>> From: Zhang Chen <chen.zh...@intel.com> > >>> > >>> This patch to fix the origin "char *data" menory leak, code style > >>> issue and add necessary check here. > >>> Reported-by: Coverity (CID 1402785) > >>> > >>> Signed-off-by: Zhang Chen <chen.zh...@intel.com> > >>> --- > >>> net/colo-compare.c | 28 +++++++++++++++++++++------- > >>> 1 file changed, 21 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/net/colo-compare.c b/net/colo-compare.c index > >>> 909dd6c6eb..ed349f5f6a 100644 > >>> --- a/net/colo-compare.c > >>> +++ b/net/colo-compare.c > >>> @@ -127,6 +127,17 @@ static int compare_chr_send(CompareState *s, > >>> uint32_t vnet_hdr_len, > >>> bool notify_remote_frame); > >>> > >>> +static bool packet_matches_str(const char *str, > >>> + uint8_t *buf, > >>> + uint32_t packet_len) { > >>> + if (packet_len <= strlen(str)) { > >>> + return false; > >>> + } > >>> + > >>> + return !memcmp(str, buf, strlen(str) + 1); > >> > >> This assumes buf is NULL terminated (you pass notify_rs->buf) which > >> is not correct I think? > > Yes, you are right. > > How about this: > > > > static bool packet_matches_str(const char *str, > > uint8_t *buf, > > uint32_t packet_len) { > > if (packet_len != strlen(str) || !buf) { > > return false; > > } > > > When can we hit !buf?
I re-checked the code, looks the "net_fill_rstate()" can ensure buf is not empty, I will remove the !buf to send next version. Thanks Zhang Chen > > Thanks > > > > > > return !memcmp(str, buf, strlen(str)); } > > > > Thanks > > Zhang Chen > > > > > >> Thanks > >> > >> > >>> +} > >>> + > >>> static void notify_remote_frame(CompareState *s) > >>> { > >>> char msg[] = "DO_CHECKPOINT"; @@ -1008,21 +1019,24 @@ static > >>> void > >> compare_notify_rs_finalize(SocketReadState *notify_rs) > >>> { > >>> CompareState *s = container_of(notify_rs, CompareState, > >>> notify_rs); > >>> > >>> - /* Get Xen colo-frame's notify and handle the message */ > >>> - char *data = g_memdup(notify_rs->buf, notify_rs->packet_len); > >>> - char msg[] = "COLO_COMPARE_GET_XEN_INIT"; > >>> + const char msg[] = "COLO_COMPARE_GET_XEN_INIT"; > >>> int ret; > >>> > >>> - if (!strcmp(data, "COLO_USERSPACE_PROXY_INIT")) { > >>> + if (packet_matches_str("COLO_USERSPACE_PROXY_INIT", > >>> + notify_rs->buf, > >>> + notify_rs->packet_len)) { > >>> ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, > >>> true); > >>> if (ret < 0) { > >>> error_report("Notify Xen COLO-frame INIT failed"); > >>> } > >>> - } > >>> - > >>> - if (!strcmp(data, "COLO_CHECKPOINT")) { > >>> + } else if (packet_matches_str("COLO_CHECKPOINT", > >>> + notify_rs->buf, > >>> + notify_rs->packet_len)) { > >>> /* colo-compare do checkpoint, flush pri packet and > >>> remove sec packet > >> */ > >>> g_queue_foreach(&s->conn_list, colo_flush_packets, s); > >>> + } else { > >>> + error_report("COLO compare got unsupported instruction '%s'", > >>> + (char *)notify_rs->buf); > >>> } > >>> } > >>>