From 3cb833fef0dbb4f72128a6b53613ebcae305a932 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Wed, 2 Dec 2020 07:43:21 -0500 Subject: [PATCH] qemu: Fix some issues in virQEMUDriverConfigLoadNVRAMEntry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Ján Tomko Signed-off-by: Ján Tomko --- src/qemu/qemu_conf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 0ee51f1cec..a33e9923b4 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -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);