[PATCH] conf: clear the acpiNodeset field after freeing
From: Daniel P. Berrangé <berrange@redhat.com> The virDomainDeviceInfoClear method does not free the struct, only its contents, so all pointer fields must be explicitly set to NULL after releasing to avoid disk of double-free. Reported by coverity: *** CID 895678: Memory - corruptions (USE_AFTER_FREE) /src/conf/domain_conf.c: 5926 in virDomainDeviceInfoParseXML() 5920 goto cleanup; 5921 5922 5923 ret = 0; 5924 cleanup: 5925 if (ret < 0)
CID 895678: Memory - corruptions (USE_AFTER_FREE) Calling "virDomainDeviceInfoClear" frees pointer "info->acpiNodeset" which has already been freed.
5926 virDomainDeviceInfoClear(info); 5927 return ret; 5928 } 5929 5930 static int 5931 virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/device_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index d08de68717..3fa7bba649 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -138,6 +138,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfo *info) VIR_FREE(info->romfile); VIR_FREE(info->loadparm); virBitmapFree(info->acpiNodeset); + info->acpiNodeset = NUll; info->isolationGroup = 0; info->isolationGroupLocked = false; } -- 2.50.1
On Tue, Sep 09, 2025 at 10:28:33 +0100, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
The virDomainDeviceInfoClear method does not free the struct, only its contents, so all pointer fields must be explicitly set to NULL after releasing to avoid disk of double-free.
Reported by coverity:
*** CID 895678: Memory - corruptions (USE_AFTER_FREE) /src/conf/domain_conf.c: 5926 in virDomainDeviceInfoParseXML() 5920 goto cleanup; 5921 5922 5923 ret = 0; 5924 cleanup: 5925 if (ret < 0)
CID 895678: Memory - corruptions (USE_AFTER_FREE) Calling "virDomainDeviceInfoClear" frees pointer "info->acpiNodeset" which has already been freed.
5926 virDomainDeviceInfoClear(info); 5927 return ret; 5928 } 5929 5930 static int 5931 virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/device_conf.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index d08de68717..3fa7bba649 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -138,6 +138,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfo *info) VIR_FREE(info->romfile); VIR_FREE(info->loadparm); virBitmapFree(info->acpiNodeset); + info->acpiNodeset = NUll;
NULL instead of NUll Also consider using g_clear_pointer(&info->acpiNodeset, virBitmapFree) instead. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Tue, Sep 09, 2025 at 11:36:42AM +0200, Peter Krempa wrote:
On Tue, Sep 09, 2025 at 10:28:33 +0100, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
The virDomainDeviceInfoClear method does not free the struct, only its contents, so all pointer fields must be explicitly set to NULL after releasing to avoid disk of double-free.
Reported by coverity:
*** CID 895678: Memory - corruptions (USE_AFTER_FREE) /src/conf/domain_conf.c: 5926 in virDomainDeviceInfoParseXML() 5920 goto cleanup; 5921 5922 5923 ret = 0; 5924 cleanup: 5925 if (ret < 0)
CID 895678: Memory - corruptions (USE_AFTER_FREE) Calling "virDomainDeviceInfoClear" frees pointer "info->acpiNodeset" which has already been freed.
5926 virDomainDeviceInfoClear(info); 5927 return ret; 5928 } 5929 5930 static int 5931 virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/device_conf.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index d08de68717..3fa7bba649 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -138,6 +138,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfo *info) VIR_FREE(info->romfile); VIR_FREE(info->loadparm); virBitmapFree(info->acpiNodeset); + info->acpiNodeset = NUll;
NULL instead of NUll
Also consider using g_clear_pointer(&info->acpiNodeset, virBitmapFree) instead.
Yes, I've done that instead.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Peter Krempa