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 <pkre...@redhat.com>
---
 tools/virsh-domain-monitor.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 0e49f5da8a..a2bf5c05fc 100644
--- 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);
-- 
2.30.2

Reply via email to