[PATCH] qemu_conf: Fix double free problem for cfg->firmwares

cfg->firmwares still points to the original memory address after being freed by virFirmwareFreeList(). As cfg get freed, it will be freed again even if cfg->nfirmwares=0 which eventually lead to crash. The patch fix it by setting cfg->firmwares to NULL explicitly after virFirmwareFreeList() returns Signed-off-by: Tuguoyi <tu.guoyi@h3c.com> --- src/qemu/qemu_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 83de26a..98593b5 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -832,6 +832,7 @@ virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg, VIR_AUTOSTRINGLIST fwList = NULL; virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); + cfg->firmwares = NULL; if (qemuFirmwareFetchConfigs(&fwList, privileged) < 0) return -1; -- 2.7.4 -- Best regards, Guoyi

On a Tuesday in 2020, Tuguoyi wrote:
cfg->firmwares still points to the original memory address after being freed by virFirmwareFreeList(). As cfg get freed, it will be freed again even if cfg->nfirmwares=0 which eventually lead to crash.
The patch fix it by setting cfg->firmwares to NULL explicitly after virFirmwareFreeList() returns
Signed-off-by: Tuguoyi <tu.guoyi@h3c.com>
Should there be a space separating your name(s)?
--- src/qemu/qemu_conf.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On a Tuesday in 2020, Ján Tomko wrote:
On a Tuesday in 2020, Tuguoyi wrote:
cfg->firmwares still points to the original memory address after being freed by virFirmwareFreeList(). As cfg get freed, it will be freed again even if cfg->nfirmwares=0 which eventually lead to crash.
The patch fix it by setting cfg->firmwares to NULL explicitly after virFirmwareFreeList() returns
Signed-off-by: Tuguoyi <tu.guoyi@h3c.com>
Should there be a space separating your name(s)? It can be changed to: Signed-off-by: Guoyi Tu<tu.guoyi@h3c.com>

-----Original Message----- From: Ján Tomko [mailto:jtomko@redhat.com] Sent: Tuesday, November 24, 2020 6:57 PM To: tuguoyi (Cloud) <tu.guoyi@h3c.com> Cc: libvir-list@redhat.com Subject: Re: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
On a Tuesday in 2020, Tuguoyi wrote:
cfg->firmwares still points to the original memory address after being freed by virFirmwareFreeList(). As cfg get freed, it will be freed again even if cfg->nfirmwares=0 which eventually lead to crash.
The patch fix it by setting cfg->firmwares to NULL explicitly after virFirmwareFreeList() returns
Signed-off-by: Tuguoyi <tu.guoyi@h3c.com>
Should there be a space separating your name(s)?
--- src/qemu/qemu_conf.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Hi there, It's my first time to submit patch to libvirt, so I'm wondering will this patch be applied to the upstream?

On 12/1/20 2:50 AM, Tuguoyi wrote:
-----Original Message----- From: Ján Tomko [mailto:jtomko@redhat.com] Sent: Tuesday, November 24, 2020 6:57 PM To: tuguoyi (Cloud) <tu.guoyi@h3c.com> Cc: libvir-list@redhat.com Subject: Re: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
On a Tuesday in 2020, Tuguoyi wrote:
cfg->firmwares still points to the original memory address after being freed by virFirmwareFreeList(). As cfg get freed, it will be freed again even if cfg->nfirmwares=0 which eventually lead to crash.
The patch fix it by setting cfg->firmwares to NULL explicitly after virFirmwareFreeList() returns
Signed-off-by: Tuguoyi <tu.guoyi@h3c.com>
Should there be a space separating your name(s)?
--- src/qemu/qemu_conf.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Hi there,
It's my first time to submit patch to libvirt, so I'm wondering will this patch be applied to the upstream?
Oh yeah, sorry. I've pushed it now: https://gitlab.com/libvirt/libvirt/-/commit/c4f4e195a14c86b7daff2c45f1cbfd23... Congratulations on your first libvirt contribution! Michal

On a Tuesday in 2020, Michal Privoznik wrote:
On 12/1/20 2:50 AM, Tuguoyi wrote:
-----Original Message----- From: Ján Tomko [mailto:jtomko@redhat.com] Sent: Tuesday, November 24, 2020 6:57 PM To: tuguoyi (Cloud) <tu.guoyi@h3c.com> Cc: libvir-list@redhat.com Subject: Re: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
On a Tuesday in 2020, Tuguoyi wrote:
cfg->firmwares still points to the original memory address after being freed by virFirmwareFreeList(). As cfg get freed, it will be freed again even if cfg->nfirmwares=0 which eventually lead to crash.
The patch fix it by setting cfg->firmwares to NULL explicitly after virFirmwareFreeList() returns
Signed-off-by: Tuguoyi <tu.guoyi@h3c.com>
Should there be a space separating your name(s)?
--- src/qemu/qemu_conf.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Hi there,
It's my first time to submit patch to libvirt, so I'm wondering will this patch be applied to the upstream?
Oh yeah, sorry. I've pushed it now:
Thank you, Jano
https://gitlab.com/libvirt/libvirt/-/commit/c4f4e195a14c86b7daff2c45f1cbfd23...
Congratulations on your first libvirt contribution!
Michal

-----Original Message----- From: Michal Privoznik [mailto:mprivozn@redhat.com] Sent: Tuesday, December 01, 2020 9:28 PM To: tuguoyi (Cloud) <tu.guoyi@h3c.com>; Ján Tomko <jtomko@redhat.com> Cc: libvir-list@redhat.com Subject: Re: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
On 12/1/20 2:50 AM, Tuguoyi wrote:
-----Original Message----- From: Ján Tomko [mailto:jtomko@redhat.com] Sent: Tuesday, November 24, 2020 6:57 PM To: tuguoyi (Cloud) <tu.guoyi@h3c.com> Cc: libvir-list@redhat.com Subject: Re: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
On a Tuesday in 2020, Tuguoyi wrote:
cfg->firmwares still points to the original memory address after being freed by virFirmwareFreeList(). As cfg get freed, it will be freed again even if cfg->nfirmwares=0 which eventually lead to crash.
The patch fix it by setting cfg->firmwares to NULL explicitly after virFirmwareFreeList() returns
Signed-off-by: Tuguoyi <tu.guoyi@h3c.com>
Should there be a space separating your name(s)?
--- src/qemu/qemu_conf.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Hi there,
It's my first time to submit patch to libvirt, so I'm wondering will this patch be applied to the upstream?
Oh yeah, sorry. I've pushed it now:
https://gitlab.com/libvirt/libvirt/-/commit/c4f4e195a14c86b7daff2c45f1cbfd2 3ac16aaa8
Congratulations on your first libvirt contribution!
Michal
Thanks a lot
participants (3)
-
Ján Tomko
-
Michal Privoznik
-
Tuguoyi