[libvirt] [PATCH] virsh: Null terminated the string memcpy from buffer explicitly
--- tools/virsh.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 98305c0..4509020 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -15069,11 +15069,13 @@ cleanup: VIR_FREE(device_type); VIR_FREE(disk_type); if (xml_buf) { - if (VIR_ALLOC_N(ret, xmlBufferLength(xml_buf)) < 0) { + int len = xmlBufferLength(xml_buf); + if (VIR_ALLOC_N(ret, len + 1) < 0) { virReportOOMError(); return NULL; } - memcpy(ret, (char *)xmlBufferContent(xml_buf), xmlBufferLength(xml_buf)); + memcpy(ret, (char *)xmlBufferContent(xml_buf), len); + ret[len] = '\0'; xmlBufferFree(xml_buf); } return ret; -- 1.7.7.3
On 06/14/2012 05:09 AM, Osier Yang wrote:
--- tools/virsh.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
A bit light on the commit message. Does this solve an actual crash, or does it just silence an overactive static code analyzer?
diff --git a/tools/virsh.c b/tools/virsh.c index 98305c0..4509020 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -15069,11 +15069,13 @@ cleanup: VIR_FREE(device_type); VIR_FREE(disk_type); if (xml_buf) { - if (VIR_ALLOC_N(ret, xmlBufferLength(xml_buf)) < 0) { + int len = xmlBufferLength(xml_buf); + if (VIR_ALLOC_N(ret, len + 1) < 0) { virReportOOMError(); return NULL; } - memcpy(ret, (char *)xmlBufferContent(xml_buf), xmlBufferLength(xml_buf)); + memcpy(ret, (char *)xmlBufferContent(xml_buf), len); + ret[len] = '\0';
At any rate, the conversion makes sense. ACK, if you'll improve the commit message. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年06月14日 19:16, Eric Blake wrote:
On 06/14/2012 05:09 AM, Osier Yang wrote:
--- tools/virsh.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
A bit light on the commit message. Does this solve an actual crash, or does it just silence an overactive static code analyzer?
It's detected by valgrind: # valgrind -v virsh change-media 3 hdc /var/lib/libvirt/images/foo2.img ==16217== 1 errors in context 1 of 12: ==16217== Invalid read of size 1 ==16217== at 0x4A07804: __GI_strlen (mc_replace_strmem.c:284) ==16217== by 0x3019F167F6: xdr_string (in /lib64/libc-2.12.so) ==16217== by 0x3033709E8D: xdr_remote_nonnull_string (remote_protocol.c:31) ==16217== by 0x303370E5CB: xdr_remote_domain_update_device_flags_args (remote_protocol.c:2028) ==16217== by 0x30337197D1: virNetMessageEncodePayload (virnetmessage.c:341) ==16217== by 0x30337135E1: virNetClientProgramCall (virnetclientprogram.c:327) ==16217== by 0x30336F1EFD: callWithFD (remote_driver.c:4586) ==16217== by 0x30336F1F7B: call (remote_driver.c:4607) ==16217== by 0x30336F42F2: remoteDomainUpdateDeviceFlags (remote_client_bodies.h:2865) ==16217== by 0x30336D46E5: virDomainUpdateDeviceFlags (libvirt.c:9457) ==16217== by 0x41AEE8: cmdChangeMedia (virsh.c:15249) ==16217== by 0x413CB4: vshCommandRun (virsh.c:18669) ==16217== Address 0x4ec5e25 is 0 bytes after a block of size 293 alloc'd ==16217== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==16217== by 0x303364F1DB: virAllocN (memory.c:129) ==16217== by 0x41A844: vshPrepareDiskXML (virsh.c:15043) ==16217== by 0x41AECC: cmdChangeMedia (virsh.c:15246) ==16217== by 0x413CB4: vshCommandRun (virsh.c:18669) ==16217== by 0x423973: main (virsh.c:20261)
diff --git a/tools/virsh.c b/tools/virsh.c index 98305c0..4509020 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -15069,11 +15069,13 @@ cleanup: VIR_FREE(device_type); VIR_FREE(disk_type); if (xml_buf) { - if (VIR_ALLOC_N(ret, xmlBufferLength(xml_buf))< 0) { + int len = xmlBufferLength(xml_buf); + if (VIR_ALLOC_N(ret, len + 1)< 0) { virReportOOMError(); return NULL; } - memcpy(ret, (char *)xmlBufferContent(xml_buf), xmlBufferLength(xml_buf)); + memcpy(ret, (char *)xmlBufferContent(xml_buf), len); + ret[len] = '\0';
At any rate, the conversion makes sense. ACK, if you'll improve the commit message.
Thanks, pushed with commit messege improved. Osier
participants (2)
-
Eric Blake -
Osier Yang