Hi On Thu, Nov 7, 2024 at 10:04 PM Dorinda Bassey <[email protected]> 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 <[email protected]>
Reviewed-by: Marc-André Lureau <[email protected]> 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
