| Message ID | 152231462116.69730.14119625999092384450.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | fix memory leaks when using object_property_get_str() | expand |
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. Reviewed-by: Cornelia Huck <cohuck@redhat.com>
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>
On 29.03.2018 11:24, Cornelia Huck 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. Or maybe remove devname from the error_report ("%s: Failed to open sev-device '%s', __func__ strerror(errno)) and move the g_free right before the if-statement? Anyway: Reviewed-by: Thomas Huth <thuth@redhat.com>
On Thu, Mar 29, 2018 at 11:10:21AM +0200, Greg Kurz wrote: > The string returned by object_property_get_str() is dynamically allocated. > > Fixes: d8575c6c0242b > Signed-off-by: Greg Kurz <groug@kaod.org> Queued, thanks.
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);
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(-)