]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
qemu: Fix some issues in virQEMUDriverConfigLoadNVRAMEntry
authorJohn Ferlan <jferlan@redhat.com>
Wed, 2 Dec 2020 12:43:21 +0000 (07:43 -0500)
committerJán Tomko <jtomko@redhat.com>
Wed, 2 Dec 2020 15:15:43 +0000 (16:15 +0100)
Commit c4f4e195 fixed a double free, but if the code returns before
we realloc the list and virFirmwareFreeList was called with cfg->nfirmwares
> 0 (e.g. during virQEMUDriverConfigDispose), then it would be rather
disastrous. So let's reinitialize that too to indicate the list is empty.

Coverity pointed out that using nvram[0] as a guard to reallocating the
list could lead to a possible NULL deref. While nvram[0] may always be
true in this case, if it wasn't then the subsequent for loop would fail.
Just reallocate always regardless - even if nfirmwares == 0 as
virFirmwareFreeList will free it for us anyway.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
src/qemu/qemu_conf.c

index 0ee51f1cec57afbd2416983c76959d8a6198ad6b..a33e9923b42f2f936be6c1319e0e56ebe18e8fe1 100644 (file)
@@ -835,6 +835,7 @@ virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg,
 
         virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
         cfg->firmwares = NULL;
+        cfg->nfirmwares = 0;
 
         if (qemuFirmwareFetchConfigs(&fwList, privileged) < 0)
             return -1;
@@ -843,13 +844,11 @@ virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg,
             VIR_WARN("Obsolete nvram variable is set while firmware metadata "
                      "files found. Note that the nvram config file variable is "
                      "going to be ignored.");
-            cfg->nfirmwares = 0;
             return 0;
         }
 
         cfg->nfirmwares = virStringListLength((const char *const *)nvram);
-        if (nvram[0])
-            cfg->firmwares = g_new0(virFirmwarePtr, cfg->nfirmwares);
+        cfg->firmwares = g_new0(virFirmwarePtr, cfg->nfirmwares);
 
         for (i = 0; nvram[i] != NULL; i++) {
             cfg->firmwares[i] = g_new0(virFirmware, 1);