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

[libvirt] [PATCH] qemu: avoid NULL derefs



The processWatchdogEvent fix is real, although it can only trigger on
OOM, since bad things happen if doCoreDump is called with a NULL
pathname argument.  The other fixes silence clang, but aren't a real
bug because virReportErrorHelper tolerates a NULL format string even
though *printf does not.

* src/qemu/qemu_driver.c (processWatchdogEvent): Exit on OOM.
(qemuDomainIsActive, qemuDomainIsPersistent, qemuDomainIsUpdated):
Provide valid message.
---

Another valid clang finding.  Maybe we whould mark
virReportErrorHelper as ATTRIBUTE_NONNULL for the format string
argument, and require all clients to pass in a valid error string
(after all, that's what the printf attritube already assumes, and
passing NULL just means that we aren't giving back any useful error
information to the user); but that can be a separate cleanup.

 src/qemu/qemu_driver.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c581cfe..82a2210 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3489,7 +3489,10 @@ static int qemuDomainIsActive(virDomainPtr dom)
     obj = virDomainFindByUUID(&driver->domains, dom->uuid);
     qemuDriverUnlock(driver);
     if (!obj) {
-        qemuReportError(VIR_ERR_NO_DOMAIN, NULL);
+        char uuidstr[VIR_UUID_STRING_BUFLEN];
+        virUUIDFormat(dom->uuid, uuidstr);
+        qemuReportError(VIR_ERR_NO_DOMAIN,
+                        _("no domain with matching uuid '%s'"), uuidstr);
         goto cleanup;
     }
     ret = virDomainObjIsActive(obj);
@@ -3510,7 +3513,10 @@ static int qemuDomainIsPersistent(virDomainPtr dom)
     obj = virDomainFindByUUID(&driver->domains, dom->uuid);
     qemuDriverUnlock(driver);
     if (!obj) {
-        qemuReportError(VIR_ERR_NO_DOMAIN, NULL);
+        char uuidstr[VIR_UUID_STRING_BUFLEN];
+        virUUIDFormat(dom->uuid, uuidstr);
+        qemuReportError(VIR_ERR_NO_DOMAIN,
+                        _("no domain with matching uuid '%s'"), uuidstr);
         goto cleanup;
     }
     ret = obj->persistent;
@@ -3531,7 +3537,10 @@ static int qemuDomainIsUpdated(virDomainPtr dom)
     obj = virDomainFindByUUID(&driver->domains, dom->uuid);
     qemuDriverUnlock(driver);
     if (!obj) {
-        qemuReportError(VIR_ERR_NO_DOMAIN, NULL);
+        char uuidstr[VIR_UUID_STRING_BUFLEN];
+        virUUIDFormat(dom->uuid, uuidstr);
+        qemuReportError(VIR_ERR_NO_DOMAIN,
+                        _("no domain with matching uuid '%s'"), uuidstr);
         goto cleanup;
     }
     ret = obj->updated;
@@ -4981,12 +4990,14 @@ static void processWatchdogEvent(void *data, void *opaque)
     case VIR_DOMAIN_WATCHDOG_ACTION_DUMP:
         {
             char *dumpfile;
-            int i;

-            i = virAsprintf(&dumpfile, "%s/%s-%u",
+            if (virAsprintf(&dumpfile, "%s/%s-%u",
                             driver->autoDumpPath,
                             wdEvent->vm->def->name,
-                            (unsigned int)time(NULL));
+                            (unsigned int)time(NULL)) < 0) {
+                virReportOOMError();
+                break;
+            }

             qemuDriverLock(driver);
             virDomainObjLock(wdEvent->vm);
-- 
1.7.4


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