1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] domain_conf: Unref video private data in virDomainVideoDefClear()

Michal Privoznik posted 1 patch 4 years, 7 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/856d1b54d61e9260797a30bfba6633fe0114613c.1569507905.git.mprivozn@redhat.com
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt] [PATCH] domain_conf: Unref video private data in virDomainVideoDefClear()
Posted by Michal Privoznik 4 years, 7 months ago 
The private data for video definition is created in
virDomainVideoDefNew() and we attempt to free it in
virDomainVideoDefFree(). This seems to work, except
the free function calls clear function which zeroes
out the whole structure and thus virObjectUnref()
which is called on private data does nothing.

2,568 bytes in 107 blocks are definitely lost in loss record 207 of 213
   at 0x4A35476: calloc (vg_replace_malloc.c:752)
   by 0x50A6048: virAllocVar (viralloc.c:346)
   by 0x513CC5A: virObjectNew (virobject.c:243)
   by 0x4DC1DEE: qemuDomainVideoPrivateNew (qemu_domain.c:1337)
   by 0x51A6BD6: virDomainVideoDefNew (domain_conf.c:2831)
   by 0x51B9F06: virDomainVideoDefParseXML (domain_conf.c:15541)
   by 0x51CB761: virDomainDefParseXML (domain_conf.c:21158)
   by 0x51C5973: virDomainDefParseNode (domain_conf.c:21708)
   by 0x51C583A: virDomainDefParse (domain_conf.c:21663)
   by 0x51C58AE: virDomainDefParseFile (domain_conf.c:21688)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/domain_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 67555c9be3..c290baf953 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2853,6 +2853,7 @@ virDomainVideoDefClear(virDomainVideoDefPtr def)
     if (def->driver)
         VIR_FREE(def->driver->vhost_user_binary);
     VIR_FREE(def->driver);
+    virObjectUnref(def->privateData);
 
     memset(def, 0, sizeof(*def));
 }
@@ -2864,7 +2865,6 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def)
         return;
 
     virDomainVideoDefClear(def);
-    virObjectUnref(def->privateData);
     VIR_FREE(def);
 }
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_conf: Unref video private data in virDomainVideoDefClear()
Posted by Erik Skultety 4 years, 7 months ago 
On Thu, Sep 26, 2019 at 04:25:05PM +0200, Michal Privoznik wrote:
> The private data for video definition is created in
> virDomainVideoDefNew() and we attempt to free it in
> virDomainVideoDefFree(). This seems to work, except
> the free function calls clear function which zeroes
> out the whole structure and thus virObjectUnref()
> which is called on private data does nothing.
>
> 2,568 bytes in 107 blocks are definitely lost in loss record 207 of 213
>    at 0x4A35476: calloc (vg_replace_malloc.c:752)
>    by 0x50A6048: virAllocVar (viralloc.c:346)
>    by 0x513CC5A: virObjectNew (virobject.c:243)
>    by 0x4DC1DEE: qemuDomainVideoPrivateNew (qemu_domain.c:1337)
>    by 0x51A6BD6: virDomainVideoDefNew (domain_conf.c:2831)
>    by 0x51B9F06: virDomainVideoDefParseXML (domain_conf.c:15541)
>    by 0x51CB761: virDomainDefParseXML (domain_conf.c:21158)
>    by 0x51C5973: virDomainDefParseNode (domain_conf.c:21708)
>    by 0x51C583A: virDomainDefParse (domain_conf.c:21663)
>    by 0x51C58AE: virDomainDefParseFile (domain_conf.c:21688)

Impressive that we haven't uncovered it sooner.

>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_conf: Unref video private data in virDomainVideoDefClear()
Posted by Michal Privoznik 4 years, 7 months ago 
On 9/26/19 4:42 PM, Erik Skultety wrote:
> On Thu, Sep 26, 2019 at 04:25:05PM +0200, Michal Privoznik wrote:
>> The private data for video definition is created in
>> virDomainVideoDefNew() and we attempt to free it in
>> virDomainVideoDefFree(). This seems to work, except
>> the free function calls clear function which zeroes
>> out the whole structure and thus virObjectUnref()
>> which is called on private data does nothing.
>>
>> 2,568 bytes in 107 blocks are definitely lost in loss record 207 of 213
>>     at 0x4A35476: calloc (vg_replace_malloc.c:752)
>>     by 0x50A6048: virAllocVar (viralloc.c:346)
>>     by 0x513CC5A: virObjectNew (virobject.c:243)
>>     by 0x4DC1DEE: qemuDomainVideoPrivateNew (qemu_domain.c:1337)
>>     by 0x51A6BD6: virDomainVideoDefNew (domain_conf.c:2831)
>>     by 0x51B9F06: virDomainVideoDefParseXML (domain_conf.c:15541)
>>     by 0x51CB761: virDomainDefParseXML (domain_conf.c:21158)
>>     by 0x51C5973: virDomainDefParseNode (domain_conf.c:21708)
>>     by 0x51C583A: virDomainDefParse (domain_conf.c:21663)
>>     by 0x51C58AE: virDomainDefParseFile (domain_conf.c:21688)
> 
> Impressive that we haven't uncovered it sooner.

That's okay, this was introduced only a few days ago in 
v5.7.0-212-g3dbf3941ad.

> 
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
> 
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
> 

Thanks, pushed now.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_conf: Unref video private data in virDomainVideoDefClear()
Posted by Erik Skultety 4 years, 7 months ago 
On Thu, Sep 26, 2019 at 05:01:24PM +0200, Michal Privoznik wrote:
> On 9/26/19 4:42 PM, Erik Skultety wrote:
> > On Thu, Sep 26, 2019 at 04:25:05PM +0200, Michal Privoznik wrote:
> > > The private data for video definition is created in
> > > virDomainVideoDefNew() and we attempt to free it in
> > > virDomainVideoDefFree(). This seems to work, except
> > > the free function calls clear function which zeroes
> > > out the whole structure and thus virObjectUnref()
> > > which is called on private data does nothing.
> > >
> > > 2,568 bytes in 107 blocks are definitely lost in loss record 207 of 213
> > >     at 0x4A35476: calloc (vg_replace_malloc.c:752)
> > >     by 0x50A6048: virAllocVar (viralloc.c:346)
> > >     by 0x513CC5A: virObjectNew (virobject.c:243)
> > >     by 0x4DC1DEE: qemuDomainVideoPrivateNew (qemu_domain.c:1337)
> > >     by 0x51A6BD6: virDomainVideoDefNew (domain_conf.c:2831)
> > >     by 0x51B9F06: virDomainVideoDefParseXML (domain_conf.c:15541)
> > >     by 0x51CB761: virDomainDefParseXML (domain_conf.c:21158)
> > >     by 0x51C5973: virDomainDefParseNode (domain_conf.c:21708)
> > >     by 0x51C583A: virDomainDefParse (domain_conf.c:21663)
> > >     by 0x51C58AE: virDomainDefParseFile (domain_conf.c:21688)
> >
> > Impressive that we haven't uncovered it sooner.
>
> That's okay, this was introduced only a few days ago in
> v5.7.0-212-g3dbf3941ad.

Ah, missed the fact that the series touched the object cleanup code as well.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.