From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
When the given uuid is already present in the hash table,
virtio_add_resource() does not add the passed VirtioSharedObject. In
this case, free it in the callers to avoid leaking memory. This fixed
the following `make check` error, when built with --enable-sanitizers:
4/166 qemu:unit / test-virtio-dmabuf ERROR 1.51s exit status 1
==7716==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 320 byte(s) in 20 object(s) allocated from:
#0 0x7f6fc16e3808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
#1 0x7f6fc1503e98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98)
#2 0x564d63cafb6b in test_add_invalid_resource ../tests/unit/test-virtio-dmabuf.c:100
#3 0x7f6fc152659d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a59d)
SUMMARY: AddressSanitizer: 320 byte(s) leaked in 20 allocation(s).
The changes at virtio_add_resource() itself are not strictly necessary
for the memleak fix, but they make it more obvious that, on an error
return, the passed object is not added to the hash.
Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
Message-Id: <c61c13f9a0c67dec473bdbfc8789c29ef26c900b.1696624734.git.quic_mathbern@quicinc.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Albert Esteve <aesteve@redhat.com>
Signed-off-by: Matheus Tavares Bernardino <<a href="mailto:quic_mathbern@quicinc.com" target="_blank">quic_mathbern@quicinc.com</a>><br>
---
hw/display/virtio-dmabuf.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
index 4a8e430f3d..3dba4577ca 100644
--- a/hw/display/virtio-dmabuf.c
+++ b/hw/display/virtio-dmabuf.c
@@ -29,7 +29,7 @@ static int uuid_equal_func(const void *lhv, const void *rhv)
static bool virtio_add_resource(QemuUUID *uuid, VirtioSharedObject *value)
{
- bool result = false;
+ bool result = true;
g_mutex_lock(&lock);
if (resource_uuids == NULL) {
@@ -39,7 +39,9 @@ static bool virtio_add_resource(QemuUUID *uuid, VirtioSharedObject *value)
g_free);
}
if (g_hash_table_lookup(resource_uuids, uuid) == NULL) {
- result = g_hash_table_insert(resource_uuids, uuid, value);
+ g_hash_table_insert(resource_uuids, uuid, value);
+ } else {
+ result = false;
}
g_mutex_unlock(&lock);
@@ -57,6 +59,9 @@ bool virtio_add_dmabuf(QemuUUID *uuid, int udmabuf_fd)
vso->type = TYPE_DMABUF;
vso->value = GINT_TO_POINTER(udmabuf_fd);
result = virtio_add_resource(uuid, vso);
+ if (!result) {
+ g_free(vso);
+ }
return result;
}
@@ -72,6 +77,9 @@ bool virtio_add_vhost_device(QemuUUID *uuid, struct vhost_dev *dev)
vso->type = TYPE_VHOST_DEV;
vso->value = dev;
result = virtio_add_resource(uuid, vso);
+ if (!result) {
+ g_free(vso);
+ }
return result;
}
--
MST