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

Tuguoyi posted 1 patch 3 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cbef30a8ad624c7f9023cf60270f3ffe@h3c.com
src/qemu/qemu_conf.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] qemu_conf: Fix double free problem for cfg->firmwares
Posted by Tuguoyi 3 years, 4 months ago
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


Re: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
Posted by Ján Tomko 3 years, 4 months ago
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
RE: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
Posted by Tuguoyi 3 years, 3 months ago
> -----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?

Re: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
Posted by Michal Privoznik 3 years, 3 months ago
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/c4f4e195a14c86b7daff2c45f1cbfd23ac16aaa8

Congratulations on your first libvirt contribution!

Michal

RE: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
Posted by Tuguoyi 3 years, 3 months ago
> -----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

Re: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
Posted by Ján Tomko 3 years, 3 months ago
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/c4f4e195a14c86b7daff2c45f1cbfd23ac16aaa8
>
>Congratulations on your first libvirt contribution!
>
>Michal
>
RE: [PATCH] qemu_conf: Fix double free problem for cfg->firmwares
Posted by Tuguoyi 3 years, 4 months ago
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>