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

["Marc-André Lureau" <marcandre.lureau@gmail.com>]

Hi

On Thu, Nov 7, 2024 at 10:04 PM Dorinda Bassey <dbassey@redhat.com> 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. 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.
>
> Fixes: faefdba847 ("hw/display: introduce virtio-dmabuf")
>
> Signed-off-by: Dorinda Bassey <dbassey@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

We missed this patch during the -rc period. Can it be included?

it fixes invalid memory access / use-after-free .

Note: I think the original intent was that the @uuid argument
ownership was passed:
virtio_add_dmabuf/virtio_add_vhost_device
 * @uuid: new resource's UUID

It could be clarified and be passed as const like getters to eventually help...

> ---
>  hw/display/virtio-dmabuf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
> index 3dba4577ca7..5e0395be77c 100644
> --- a/hw/display/virtio-dmabuf.c
> +++ b/hw/display/virtio-dmabuf.c
> @@ -35,11 +35,13 @@ static bool virtio_add_resource(QemuUUID *uuid, VirtioSharedObject *value)
>      if (resource_uuids == NULL) {
>          resource_uuids = g_hash_table_new_full(qemu_uuid_hash,
>                                                 uuid_equal_func,
> -                                               NULL,
> +                                               g_free,
>                                                 g_free);
>      }
>      if (g_hash_table_lookup(resource_uuids, uuid) == NULL) {
> -        g_hash_table_insert(resource_uuids, uuid, value);
> +        g_hash_table_insert(resource_uuids,
> +                            g_memdup2(uuid, sizeof(*uuid)),
> +                            value);
>      } else {
>          result = false;
>      }
> --
> 2.47.0
>
>


-- 
Marc-André Lureau