On Thu, Nov 7, 2024 at 2:43 PM Stefano Garzarella <sgarz...@redhat.com> wrote:
>
> On Thu, Nov 07, 2024 at 01:52:01PM +0100, Dorinda Bassey wrote:
> >In `virtio_add_resource` function, the UUID used as a key for
> >`g_hash_table_insert` was temporary, which could lead to
> >invalid lookups when accessed later. This patch ensures that
> >the UUID remains valid by duplicating it into a newly allocated
> >memory space (persistent_uuid). The value is then inserted into
> >the hash table with this persistent UUID key to ensure that the
> >key stored in the hash table remains valid as long as the hash
> >table entry exists.
>
> It's a fix right, so maybe better to add a Fixes tag:
>
> Fixes: faefdba847 ("hw/display: introduce virtio-dmabuf")
>
> >
> >Signed-off-by: Dorinda Bassey <dbas...@redhat.com>
> >---
> > hw/display/virtio-dmabuf.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
> >index 3dba4577ca7..4353970bc87 100644
> >--- a/hw/display/virtio-dmabuf.c
> >+++ b/hw/display/virtio-dmabuf.c
> >@@ -39,7 +39,12 @@ static bool virtio_add_resource(QemuUUID *uuid,
> >VirtioSharedObject *value)
> > g_free);
> > }
> > if (g_hash_table_lookup(resource_uuids, uuid) == NULL) {
> >- g_hash_table_insert(resource_uuids, uuid, value);
> >+ QemuUUID *persistent_uuid = g_memdup2(uuid, sizeof(QemuUUID));
>
> Since now we allocate memory for the key, we should provide the
> `key_destroy_func` when calling g_hash_table_new_full(), otherwise
> this new memory will not be de-allocated.
>
> >+ if (persistent_uuid == NULL) {
>
> IIUC it can be null, only if `uuid` was null since glib memory
> API usually terminates the application if memory allocation fails,
> see https://docs.gtk.org/glib/memory.html
>
> So maybe we can just do:
> g_hash_table_insert(resource_uuids, g_memdup2(uuid,
> sizeof(QemuUUID)),
> value);
Or even better:
g_hash_table_insert(resource_uuids, g_memdup2(uuid, sizeof(*uuid)),
value);
Thanks,
Stefano
>
> >+ result = false;
> >+ } else {
> >+ g_hash_table_insert(resource_uuids, persistent_uuid, value);
> >+ }
> > } else {
> > result = false;
> > }
> >--
> >2.47.0
> >
