Changeset
src/conf/domain_conf.c         |  3 ++-
src/qemu/qemu_monitor.h        |  2 +-
src/vbox/vbox_common.c         | 28 +++++++++++++++-------------
tests/libxlxml2domconfigtest.c |  4 +++-
tests/qemumonitorjsontest.c    |  2 +-
5 files changed, 22 insertions(+), 17 deletions(-)
Git apply log
Switched to a new branch '20180608171019.32167-1-jferlan@redhat.com'
Applying: vbox: Fix resource leak
Applying: vbox: Fix resource leak
Applying: qemu: Fix Coverity build for qemu_monitor
Applying: test: Fix resource leak in qemumonitorjsontest
Applying: test: Check return status for libxlxml2domconfigtest
Applying: conf: Check error from virXMLFormatElement call
To https://github.com/patchew-project/libvirt
 + 4877004...373099d patchew/20180608171019.32167-1-jferlan@redhat.com -> patchew/20180608171019.32167-1-jferlan@redhat.com (forced update)
Test passed: syntax-check

loading

[libvirt] [PATCH 0/6] Some Coverity patches
Posted by John Ferlan, 2 weeks ago
Resolve a few things that have built up previous and a couple
that are more recent.

John Ferlan (6):
  vbox: Fix resource leak
  vbox: Fix resource leak
  qemu: Fix Coverity build for qemu_monitor
  test: Fix resource leak in qemumonitorjsontest
  test: Check return status for libxlxml2domconfigtest
  conf: Check error from virXMLFormatElement call

 src/conf/domain_conf.c         |  3 ++-
 src/qemu/qemu_monitor.h        |  2 +-
 src/vbox/vbox_common.c         | 28 +++++++++++++++-------------
 tests/libxlxml2domconfigtest.c |  4 +++-
 tests/qemumonitorjsontest.c    |  2 +-
 5 files changed, 22 insertions(+), 17 deletions(-)

-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] Some Coverity patches
Posted by Katerina Koukiou, 1 week ago
On Fri, Jun 08, 2018 at 01:10:13PM -0400, John Ferlan wrote:
> Resolve a few things that have built up previous and a couple
> that are more recent.
> 
> John Ferlan (6):
>   vbox: Fix resource leak
>   vbox: Fix resource leak
>   qemu: Fix Coverity build for qemu_monitor
>   test: Fix resource leak in qemumonitorjsontest
>   test: Check return status for libxlxml2domconfigtest
>   conf: Check error from virXMLFormatElement call
> 
>  src/conf/domain_conf.c         |  3 ++-
>  src/qemu/qemu_monitor.h        |  2 +-
>  src/vbox/vbox_common.c         | 28 +++++++++++++++-------------
>  tests/libxlxml2domconfigtest.c |  4 +++-
>  tests/qemumonitorjsontest.c    |  2 +-
>  5 files changed, 22 insertions(+), 17 deletions(-)

Reviewed-by: Katerina Koukiou <kkoukiou@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] vbox: Fix resource leak
Posted by John Ferlan, 2 weeks ago
Need to free the allocated hardDiskToOpen array. The contents of the
array are just pointers returned by virVBoxSnapshotConfHardDiskByLocation
and not allocated AFAICT so they don't need to also be freed as well.

Found by Coverity

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/vbox/vbox_common.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 72a24a3464..0e7fe06748 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4627,6 +4627,8 @@ vboxSnapshotRedefine(virDomainPtr dom,
     int realReadOnlyDisksPathSize = 0;
     virVBoxSnapshotConfSnapshotPtr newSnapshotPtr = NULL;
     unsigned char snapshotUuid[VIR_UUID_BUFLEN];
+    virVBoxSnapshotConfHardDiskPtr *hardDiskToOpen = NULL;
+    size_t hardDiskToOpenSize = 0;
     char **searchResultTab = NULL;
     ssize_t resultSize = 0;
     int it = 0;
@@ -5080,8 +5082,6 @@ vboxSnapshotRedefine(virDomainPtr dom,
      */
     for (it = 0; it < def->dom->ndisks; it++) {
         char *location = NULL;
-        virVBoxSnapshotConfHardDiskPtr *hardDiskToOpen = NULL;
-        size_t hardDiskToOpenSize = 0;
 
         location = def->dom->disks[it]->src->path;
         if (!location)
@@ -5394,8 +5394,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
         if (!location)
             goto cleanup;
 
-        virVBoxSnapshotConfHardDiskPtr *hardDiskToOpen = NULL;
-        size_t hardDiskToOpenSize = virVBoxSnapshotConfDiskListToOpen(snapshotMachineDesc,
+        hardDiskToOpenSize = virVBoxSnapshotConfDiskListToOpen(snapshotMachineDesc,
                                                    &hardDiskToOpen, location);
         for (jt = 0; jt < hardDiskToOpenSize; jt++) {
             IMedium *medium = NULL;
@@ -5459,6 +5458,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
     virStringListFree(realReadOnlyDisksPath);
     virStringListFree(realReadWriteDisksPath);
     virStringListFree(searchResultTab);
+    VIR_FREE(hardDiskToOpen);
     VIR_FREE(newSnapshotPtr);
     VIR_FREE(machineLocationPath);
     VIR_FREE(nameTmpUse);
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] vbox: Fix resource leak
Posted by John Ferlan, 2 weeks ago
The @disk was allocated, filled in, and consumed on the normal path,
but for error/cleanup paths it would be leaked.  Rename to newHardDisk
and manage properly.

Found by Coverity

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/vbox/vbox_common.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 0e7fe06748..664650f217 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4629,6 +4629,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
     unsigned char snapshotUuid[VIR_UUID_BUFLEN];
     virVBoxSnapshotConfHardDiskPtr *hardDiskToOpen = NULL;
     size_t hardDiskToOpenSize = 0;
+    virVBoxSnapshotConfHardDiskPtr newHardDisk = NULL;
     char **searchResultTab = NULL;
     ssize_t resultSize = 0;
     int it = 0;
@@ -5236,7 +5237,6 @@ vboxSnapshotRedefine(virDomainPtr dom,
             PRUnichar *newLocation = NULL;
             char *newLocationUtf8 = NULL;
             resultCodeUnion resultCode;
-            virVBoxSnapshotConfHardDiskPtr disk = NULL;
             char *uuid = NULL;
             char *format = NULL;
             char *tmp = NULL;
@@ -5301,11 +5301,11 @@ vboxSnapshotRedefine(virDomainPtr dom,
             }
             VBOX_RELEASE(progress);
             /*
-             * The differential disk is created, we add it to the media registry and the
-             * machine storage controllers.
+             * The differential newHardDisk is created, we add it to the
+             * media registry and the machine storage controllers.
              */
 
-            if (VIR_ALLOC(disk) < 0)
+            if (VIR_ALLOC(newHardDisk) < 0)
                 goto cleanup;
 
             rc = gVBoxAPI.UIMedium.GetId(newMedium, &iid);
@@ -5316,24 +5316,25 @@ vboxSnapshotRedefine(virDomainPtr dom,
                 goto cleanup;
             }
             gVBoxAPI.UIID.vboxIIDToUtf8(data, &iid, &uuid);
-            disk->uuid = uuid;
+            newHardDisk->uuid = uuid;
             vboxIIDUnalloc(&iid);
 
-            if (VIR_STRDUP(disk->location, newLocationUtf8) < 0)
+            if (VIR_STRDUP(newHardDisk->location, newLocationUtf8) < 0)
                 goto cleanup;
 
             rc = gVBoxAPI.UIMedium.GetFormat(newMedium, &formatUtf16);
             VBOX_UTF16_TO_UTF8(formatUtf16, &format);
-            disk->format = format;
+            newHardDisk->format = format;
             VBOX_UTF16_FREE(formatUtf16);
 
-            if (virVBoxSnapshotConfAddHardDiskToMediaRegistry(disk,
+            if (virVBoxSnapshotConfAddHardDiskToMediaRegistry(newHardDisk,
                                            snapshotMachineDesc->mediaRegistry,
                                            parentUuid) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("Unable to add hard disk to the media registry"));
                 goto cleanup;
             }
+            newHardDisk = NULL;  /* Consumed by above */
             /*Adding the fake disk to the machine storage controllers*/
 
             resultSize = virStringSearch(snapshotMachineDesc->storageController,
@@ -5348,7 +5349,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
 
             tmp = virStringReplace(snapshotMachineDesc->storageController,
                                    searchResultTab[it],
-                                   disk->uuid);
+                                   uuid);
             VIR_FREE(snapshotMachineDesc->storageController);
             if (!tmp)
                 goto cleanup;
@@ -5458,6 +5459,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
     virStringListFree(realReadOnlyDisksPath);
     virStringListFree(realReadWriteDisksPath);
     virStringListFree(searchResultTab);
+    virVboxSnapshotConfHardDiskFree(newHardDisk);
     VIR_FREE(hardDiskToOpen);
     VIR_FREE(newSnapshotPtr);
     VIR_FREE(machineLocationPath);
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] qemu: Fix Coverity build for qemu_monitor
Posted by John Ferlan, 2 weeks ago
Commit id '7ef0471bf' added a new parameter to qemuMonitorOpen,
but didn't update the ATTTRIBUTE_NONNULL for the @cb (param 5).

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/qemu/qemu_monitor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 43843721bf..d01266ff86 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -317,7 +317,7 @@ qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm,
                                unsigned long long timeout,
                                qemuMonitorCallbacksPtr cb,
                                void *opaque)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(6);
 qemuMonitorPtr qemuMonitorOpenFD(virDomainObjPtr vm,
                                  int sockfd,
                                  bool json,
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] test: Fix resource leak in qemumonitorjsontest
Posted by John Ferlan, 2 weeks ago
Introduced by commmit id 37bd4571c. Need to goto cleanup and
not return directly.

Found by Coverity

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 tests/qemumonitorjsontest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 3b494a1dba..2eefd06b6e 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2693,7 +2693,7 @@ testQemuMonitorCPUInfo(const void *opaque)
 
     vm = qemuMonitorTestGetDomainObj(test);
     if (!vm)
-        return -1;
+        goto cleanup;
 
     rc = qemuMonitorGetCPUInfo(qemuMonitorTestGetMonitor(test),
                                &vcpus, data->maxvcpus, true, data->fast);
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] test: Check return status for libxlxml2domconfigtest
Posted by John Ferlan, 2 weeks ago
Commit id d8e8b63d introduced the test, but neglected to check for
error from virTestLoadFile in testCompareXMLToDomConfig.

Found by Coverity

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 tests/libxlxml2domconfigtest.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index 0d2a7385e5..54a92cc959 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -104,7 +104,9 @@ testCompareXMLToDomConfig(const char *xmlfile,
         goto cleanup;
     }
 
-    virTestLoadFile(jsonfile, &tempjson);
+    if (virTestLoadFile(jsonfile, &tempjson) < 0)
+        goto cleanup;
+
     if (libxl_domain_config_from_json(cfg->ctx, &expectconfig, tempjson) != 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        "Failed to create libxl_domain_config from JSON doc");
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] conf: Check error from virXMLFormatElement call
Posted by John Ferlan, 2 weeks ago
Commit id 1bd5a08d added a call to virXMLFormatElement without
also checking the return status.

Found by Coverity.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/conf/domain_conf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ab93bb7b45..1157c7dd93 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -27522,7 +27522,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
                                           unit, short_size);
                     }
 
-                    virXMLFormatElement(buf, "smm", &attrBuf, &childBuf);
+                    if (virXMLFormatElement(buf, "smm", &attrBuf, &childBuf) < 0)
+                        goto error;
                 }
 
                 break;
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list