[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] qemu: avoid leaking uninit data from hotplug to dumpxml

Detected by Coverity.  Both text and JSON monitors set only the
bus and unit fields, which means driveAddr.controller spends
life as garbage on the stack, and is then memcpy()'d into the
in-memory representation which the user can see via dumpxml.

* src/qemu/qemu_hotplug.c (qemuDomainAttachSCSIDisk): Only copy
defined fields.

I have to admit that Coverity is good - it took me several minutes
to follow the trail down to qemu_monitor_{text,json}.c and prove to
myself that driveAddr.controller really is untouched on success.

I didn't actually try to exploit this one - it depends on whatever
is already on the stack, and your compiler optimization levels,
before you would ever see dumpxml giving bogus information in
the <address controller='garbage'> field of the hotplugged <disk>.

 src/qemu/qemu_hotplug.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bfa524b..037f4aa 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -505,7 +505,8 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
             /* XXX we should probably validate that the addr matches
              * our existing defined addr instead of overwriting */
             disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
-            memcpy(&disk->info.addr.drive, &driveAddr, sizeof(driveAddr));
+            disk->info.addr.drive.bus = driveAddr.bus;
+            disk->info.addr.drive.unit = driveAddr.unit;
     qemuDomainObjExitMonitorWithDriver(driver, vm);

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]