On Tue, 2 Dec 2025 at 14:51, Marc-André Lureau
<[email protected]> wrote:
>
> 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?

I guess we missed this in several releases since it was sent 1 year ago :-)

BTW I think the main issue here was not ccing Michael (now in CC):

$ ./scripts/get_maintainer.pl -f hw/display/virtio-dmabuf.c
Albert Esteve <[email protected]> (supporter:virtio-dmabuf)
"Michael S. Tsirkin" <[email protected]> (supporter:virtio)
[email protected] (open list:All patches CC here)

So, I'm not sure if it's better to rebase and resend (including the
R-b) with the right maintainers in CC.

Stefano

>
> 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
>


Reply via email to