[PATCH 0/2] domain_conf: Don't leak def->os.firmwareFeatures

See 2/2. Michal Prívozník (2): domain_conf: Separate virDomainOS clear into a function domain_conf: Don't leak def->os.firmwareFeatures src/conf/domain_conf.c | 55 ++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 23 deletions(-) -- 2.26.2

The virDomainDefFree() function frees individual members of virDomainDef struct. The function is already long enough, move code that handles def->os member into a separate function. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 54 ++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 95602ae57e..3aefa8f335 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3527,6 +3527,36 @@ virDomainSEVDefFree(virDomainSEVDefPtr def) g_free(def); } +static void +virDomainOSDefClear(virDomainOSDef *os) +{ + size_t i; + + g_free(os->machine); + g_free(os->init); + for (i = 0; os->initargv && os->initargv[i]; i++) + g_free(os->initargv[i]); + g_free(os->initargv); + for (i = 0; os->initenv && os->initenv[i]; i++) { + g_free(os->initenv[i]->name); + g_free(os->initenv[i]->value); + g_free(os->initenv[i]); + } + g_free(os->initdir); + g_free(os->inituser); + g_free(os->initgroup); + g_free(os->initenv); + g_free(os->kernel); + g_free(os->initrd); + g_free(os->cmdline); + g_free(os->dtb); + g_free(os->root); + g_free(os->slic_table); + virDomainLoaderDefFree(os->loader); + g_free(os->bootloader); + g_free(os->bootloaderArgs); +} + void virDomainDefFree(virDomainDefPtr def) { @@ -3640,29 +3670,7 @@ void virDomainDefFree(virDomainDefPtr def) g_free(def->idmap.uidmap); g_free(def->idmap.gidmap); - g_free(def->os.machine); - g_free(def->os.init); - for (i = 0; def->os.initargv && def->os.initargv[i]; i++) - g_free(def->os.initargv[i]); - g_free(def->os.initargv); - for (i = 0; def->os.initenv && def->os.initenv[i]; i++) { - g_free(def->os.initenv[i]->name); - g_free(def->os.initenv[i]->value); - g_free(def->os.initenv[i]); - } - g_free(def->os.initdir); - g_free(def->os.inituser); - g_free(def->os.initgroup); - g_free(def->os.initenv); - g_free(def->os.kernel); - g_free(def->os.initrd); - g_free(def->os.cmdline); - g_free(def->os.dtb); - g_free(def->os.root); - g_free(def->os.slic_table); - virDomainLoaderDefFree(def->os.loader); - g_free(def->os.bootloader); - g_free(def->os.bootloaderArgs); + virDomainOSDefClear(&def->os); virDomainClockDefClear(&def->clock); -- 2.26.2

The firmwareFeatures member of virDomainOSDef struct is allocated in virDomainDefParseBootFirmwareOptions() but never freed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3aefa8f335..9bf12ff599 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3532,6 +3532,7 @@ virDomainOSDefClear(virDomainOSDef *os) { size_t i; + g_free(os->firmwareFeatures); g_free(os->machine); g_free(os->init); for (i = 0; os->initargv && os->initargv[i]; i++) -- 2.26.2

On Tue, Mar 23, 2021 at 11:58:47AM +0100, Michal Privoznik wrote:
See 2/2.
Michal Prívozník (2): domain_conf: Separate virDomainOS clear into a function domain_conf: Don't leak def->os.firmwareFeatures
src/conf/domain_conf.c | 55 ++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 23 deletions(-)
Oops, thanks for fixing this. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Michal Privoznik
-
Pavel Hrdina