cmdDomBlkError: Fix crash when initial call to virDomainGetDiskErrors fails

virDomainGetDiskErrors uses the weird semantics where we make the
caller query for the number of elements and then pass pre-allocated
structure.

The cleanup section errorneously used the 'count' variable to free the
allocated elements for the API but 'count' can be '-1' in cases when the
API returns failure, thus attempting to free beyond the end of the
array.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/155
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 0e49f5da8aaa5c348705853440b005b6167d9649..a2bf5c05fcd9027050435792e0efe3445de765d8 100644 (file)
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1220,7 +1220,7 @@ cmdDomBlkError(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
     virDomainDiskErrorPtr disks = NULL;
-    unsigned int ndisks;
+    unsigned int ndisks = 0;
     size_t i;
     int count;
     bool ret = false;
@@ -1230,10 +1230,10 @@ cmdDomBlkError(vshControl *ctl, const vshCmd *cmd)
 
     if ((count = virDomainGetDiskErrors(dom, NULL, 0, 0)) < 0)
         goto cleanup;
-    ndisks = count;
 
-    if (ndisks) {
-        disks = g_new0(virDomainDiskError, ndisks);
+    if (count > 0) {
+        disks = g_new0(virDomainDiskError, count);
+        ndisks = count;
 
         if ((count = virDomainGetDiskErrors(dom, disks, ndisks, 0)) == -1)
             goto cleanup;
@@ -1252,7 +1252,7 @@ cmdDomBlkError(vshControl *ctl, const vshCmd *cmd)
     ret = true;
 
  cleanup:
-    for (i = 0; i < count; i++)
+    for (i = 0; i < ndisks; i++)
         VIR_FREE(disks[i].disk);
     VIR_FREE(disks);
     virshDomainFree(dom);