[PATCH] libxl: Free data returned from libxl_userdata_retrieve

Jim Fehlig posted 1 patch 2 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211109233414.13132-1-jfehlig@suse.com
src/libxl/libxl_driver.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] libxl: Free data returned from libxl_userdata_retrieve
Posted by Jim Fehlig 2 years, 4 months ago
Found via valgrind

==15016== 3,701 bytes in 2 blocks are definitely lost in loss record 975 of 1,009
==15016==    at 0x4C2A2AF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==15016==    by 0x1FCD30CB: libxl_read_file_contents (in /usr/lib64/libxenlight.so.4.12.0)
==15016==    by 0x1FCCA58A: ??? (in /usr/lib64/libxenlight.so.4.12.0)
==15016==    by 0x1FCCA6C2: libxl_userdata_retrieve (in /usr/lib64/libxenlight.so.4.12.0)
==15016==    by 0x1FA42A5A: libxlReconnectDomain (libxl_driver.c:394)
==15016==    by 0x53BAC99: virDomainObjListHelper (virdomainobjlist.c:802)
==15016==    by 0x530842F: virHashForEach (virhash.c:575)
==15016==    by 0x53BC0E0: virDomainObjListForEach (virdomainobjlist.c:817)
==15016==    by 0x1FA423C4: libxlReconnectDomains (libxl_driver.c:468)
==15016==    by 0x1FA423C4: libxlStateInitialize (libxl_driver.c:778)
==15016==    by 0x54E8E9E: virStateInitialize (libvirt.c:657)
==15016==    by 0x12DBFA: daemonRunStateInit (remote_daemon.c:797)
==15016==    by 0x535BF79: virThreadHelper (virthread.c:206)

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7ea157f9c4..1a9bf8f186 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -461,6 +461,7 @@ libxlReconnectDomain(virDomainObj *vm,
 
  cleanup:
     libxl_dominfo_dispose(&d_info);
+    VIR_FREE(data);
     virObjectUnlock(vm);
     virObjectUnref(vm);
     virObjectUnref(cfg);
-- 
2.33.0


Re: [PATCH] libxl: Free data returned from libxl_userdata_retrieve
Posted by Michal Prívozník 2 years, 4 months ago
On 11/10/21 12:34 AM, Jim Fehlig wrote:
> Found via valgrind
> 
> ==15016== 3,701 bytes in 2 blocks are definitely lost in loss record 975 of 1,009
> ==15016==    at 0x4C2A2AF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==15016==    by 0x1FCD30CB: libxl_read_file_contents (in /usr/lib64/libxenlight.so.4.12.0)
> ==15016==    by 0x1FCCA58A: ??? (in /usr/lib64/libxenlight.so.4.12.0)
> ==15016==    by 0x1FCCA6C2: libxl_userdata_retrieve (in /usr/lib64/libxenlight.so.4.12.0)
> ==15016==    by 0x1FA42A5A: libxlReconnectDomain (libxl_driver.c:394)
> ==15016==    by 0x53BAC99: virDomainObjListHelper (virdomainobjlist.c:802)
> ==15016==    by 0x530842F: virHashForEach (virhash.c:575)
> ==15016==    by 0x53BC0E0: virDomainObjListForEach (virdomainobjlist.c:817)
> ==15016==    by 0x1FA423C4: libxlReconnectDomains (libxl_driver.c:468)
> ==15016==    by 0x1FA423C4: libxlStateInitialize (libxl_driver.c:778)
> ==15016==    by 0x54E8E9E: virStateInitialize (libvirt.c:657)
> ==15016==    by 0x12DBFA: daemonRunStateInit (remote_daemon.c:797)
> ==15016==    by 0x535BF79: virThreadHelper (virthread.c:206)
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/libxl/libxl_driver.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 7ea157f9c4..1a9bf8f186 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -461,6 +461,7 @@ libxlReconnectDomain(virDomainObj *vm,
>  
>   cleanup:
>      libxl_dominfo_dispose(&d_info);
> +    VIR_FREE(data);
>      virObjectUnlock(vm);
>      virObjectUnref(vm);
>      virObjectUnref(cfg);
> 

Alternatively, data can be declared as g_autofree:

    g_autofree uint8_t *data = NULL;

Regardless of what to decide to go with:

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

Michal

Re: [PATCH] libxl: Free data returned from libxl_userdata_retrieve
Posted by Jim Fehlig 2 years, 4 months ago
On 11/10/21 00:46, Michal Prívozník wrote:
> On 11/10/21 12:34 AM, Jim Fehlig wrote:
>> Found via valgrind
>>
>> ==15016== 3,701 bytes in 2 blocks are definitely lost in loss record 975 of 1,009
>> ==15016==    at 0x4C2A2AF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==15016==    by 0x1FCD30CB: libxl_read_file_contents (in /usr/lib64/libxenlight.so.4.12.0)
>> ==15016==    by 0x1FCCA58A: ??? (in /usr/lib64/libxenlight.so.4.12.0)
>> ==15016==    by 0x1FCCA6C2: libxl_userdata_retrieve (in /usr/lib64/libxenlight.so.4.12.0)
>> ==15016==    by 0x1FA42A5A: libxlReconnectDomain (libxl_driver.c:394)
>> ==15016==    by 0x53BAC99: virDomainObjListHelper (virdomainobjlist.c:802)
>> ==15016==    by 0x530842F: virHashForEach (virhash.c:575)
>> ==15016==    by 0x53BC0E0: virDomainObjListForEach (virdomainobjlist.c:817)
>> ==15016==    by 0x1FA423C4: libxlReconnectDomains (libxl_driver.c:468)
>> ==15016==    by 0x1FA423C4: libxlStateInitialize (libxl_driver.c:778)
>> ==15016==    by 0x54E8E9E: virStateInitialize (libvirt.c:657)
>> ==15016==    by 0x12DBFA: daemonRunStateInit (remote_daemon.c:797)
>> ==15016==    by 0x535BF79: virThreadHelper (virthread.c:206)
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>   src/libxl/libxl_driver.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 7ea157f9c4..1a9bf8f186 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -461,6 +461,7 @@ libxlReconnectDomain(virDomainObj *vm,
>>   
>>    cleanup:
>>       libxl_dominfo_dispose(&d_info);
>> +    VIR_FREE(data);
>>       virObjectUnlock(vm);
>>       virObjectUnref(vm);
>>       virObjectUnref(cfg);
>>
> 
> Alternatively, data can be declared as g_autofree:
> 
>      g_autofree uint8_t *data = NULL;

Yes, that is better. I must try harder to break old habits :-).

> Regardless of what to decide to go with:
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Thanks!
Jim