Re: [PATCH] virtio-dmabuf: Ensure UUID persistence for hash table insertion

[Stefano Garzarella]

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 <dbassey@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);

> +            result = false;
> +        } else {
> +            g_hash_table_insert(resource_uuids, persistent_uuid, value);
> +        }
>     } else {
>         result = false;
>     }
> --
> 2.47.0