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
disasterous. So let's reinitialze 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@xxxxxxxxxx>
---
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 cbdde0c0dc..690cfd39f9 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);
--
2.28.0
