Re: [Qemu-devel] [PATCH 3/3] sev/i386: fix memory leak in sev_guest_init()

[Greg Kurz <groug@kaod.org>]

On Thu, 29 Mar 2018 11:24:16 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Thu, 29 Mar 2018 11:10:21 +0200
> Greg Kurz <groug@kaod.org> wrote:
> 
> > The string returned by object_property_get_str() is dynamically allocated.
> > 
> > Fixes: d8575c6c0242b
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  target/i386/sev.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > index 019d84cef2c7..c01167143f1c 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> > @@ -748,9 +748,11 @@ sev_guest_init(const char *id)
> >      if (s->sev_fd < 0) {
> >          error_report("%s: Failed to open %s '%s'", __func__,
> >                       devname, strerror(errno));
> > -        goto err;
> >      }
> >      g_free(devname);
> > +    if (s->sev_fd < 0) {
> > +        goto err;
> > +    }
> >  
> >      ret = sev_platform_ioctl(s->sev_fd, SEV_PLATFORM_STATUS, &status,
> >                               &fw_error);
> >   
> 
> I would probably add an extra g_free(devname) right before the goto
> err, but this works as well, obviously.
> 

An alternative could have been to use glib's g_autofree macro, which
is especially designed to handle the case where an allocated buffer is
put in a local variable and we want g_free() to be called when the
variable goes out of scope. Unfortunately it requires glib >= 2.44 and
we're still at 2.22 AFAIK.

https://developer.gnome.org/glib/unstable/glib-Miscellaneous-Macros.html#g-autofree

> Reviewed-by: Cornelia Huck <cohuck@redhat.com>