[Qemu-devel] [PATCH] spice: fix memory leak

[Qemu-devel] [PATCH] spice: fix memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

If errors happen for middle items of channel_list,
qmp_query_spice_channels() return NULL, and varriable
cur_item going out of scope leaks the storage it points to.
Let's check for errors in advance avoid memory leak.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 ui/spice-core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6467fa4..8a74afe 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -381,15 +381,17 @@ static SpiceChannelList *qmp_query_spice_channels(void)
     ChannelList *item;
 
     QTAILQ_FOREACH(item, &channel_list, link) {
-        SpiceChannelList *chan;
-        char host[NI_MAXHOST], port[NI_MAXSERV];
-        struct sockaddr *paddr;
-        socklen_t plen;
-
         if (!(item->info->flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT)) {
             error_report("invalid channel event");
             return NULL;
         }
+    }
+
+    QTAILQ_FOREACH(item, &channel_list, link) {
+        SpiceChannelList *chan;
+        char host[NI_MAXHOST], port[NI_MAXSERV];
+        struct sockaddr *paddr;
+        socklen_t plen;
 
         chan = g_malloc0(sizeof(*chan));
         chan->value = g_malloc0(sizeof(*chan->value));
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH] spice: fix memory leak

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

On 12/04/2014 08:31 PM, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> If errors happen for middle items of channel_list,
> qmp_query_spice_channels() return NULL, and varriable

s/return/returns/
s/varriable/the variable/

> cur_item going out of scope leaks the storage it points to.
> Let's check for errors in advance avoid memory leak.

s/avoid/to avoid a/

> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  ui/spice-core.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 

Maintainer can fix up commit message typos.
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH] spice: fix memory leak

[Gonglei]

On 2014/12/5 12:59, Eric Blake wrote:

> On 12/04/2014 08:31 PM, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> If errors happen for middle items of channel_list,
>> qmp_query_spice_channels() return NULL, and varriable
> 
> s/return/returns/
> s/varriable/the variable/
> 
>> cur_item going out of scope leaks the storage it points to.
>> Let's check for errors in advance avoid memory leak.
> 
> s/avoid/to avoid a/
> 
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>  ui/spice-core.c | 12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>
> 
> Maintainer can fix up commit message typos.
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Thanks, Eric :)

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH] spice: fix memory leak

[Gerd Hoffmann]

On Fr, 2014-12-05 at 11:31 +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> If errors happen for middle items of channel_list,
> qmp_query_spice_channels() return NULL, and varriable
> cur_item going out of scope leaks the storage it points to.
> Let's check for errors in advance avoid memory leak.

I think we can simply turn that into an assert().  The flag is a
compatibility thing for older spice-server versions.  Meanwhile our
minimum spice version requirement is new enough that we should never
ever see this error, and if we do something went very seriously wrong.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] spice: fix memory leak

[Gonglei]

On 2014/12/5 15:17, Gerd Hoffmann wrote:

> On Fr, 2014-12-05 at 11:31 +0800, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> If errors happen for middle items of channel_list,
>> qmp_query_spice_channels() return NULL, and varriable
>> cur_item going out of scope leaks the storage it points to.
>> Let's check for errors in advance avoid memory leak.
> 
> I think we can simply turn that into an assert().  The flag is a
> compatibility thing for older spice-server versions.  Meanwhile our
> minimum spice version requirement is new enough that we should never
> ever see this error, and if we do something went very seriously wrong.
> 
Fine. Would you like version 2 ?


Regards,
-Gonglei

Re: [Qemu-devel] [PATCH] spice: fix memory leak

[Gerd Hoffmann]

On Fr, 2014-12-05 at 15:22 +0800, Gonglei wrote:
> On 2014/12/5 15:17, Gerd Hoffmann wrote:
> 
> > On Fr, 2014-12-05 at 11:31 +0800, arei.gonglei@huawei.com wrote:
> >> From: Gonglei <arei.gonglei@huawei.com>
> >>
> >> If errors happen for middle items of channel_list,
> >> qmp_query_spice_channels() return NULL, and varriable
> >> cur_item going out of scope leaks the storage it points to.
> >> Let's check for errors in advance avoid memory leak.
> > 
> > I think we can simply turn that into an assert().  The flag is a
> > compatibility thing for older spice-server versions.  Meanwhile our
> > minimum spice version requirement is new enough that we should never
> > ever see this error, and if we do something went very seriously wrong.
> > 
> Fine. Would you like version 2 ?

yes, please.

thanks,
  Gerd

[Qemu-devel] [PATCH] spice: fix memory leak

[Gerd Hoffmann]

Found by coverity.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 monitor.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/monitor.c b/monitor.c
index c3cc060..29d7e5b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1101,6 +1101,7 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
     if (strcmp(protocol, "spice") == 0) {
         if (!qemu_using_spice(&err)) {
             qerror_report_err(err);
+            error_free(local_err);
             return -1;
         }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] spice: fix memory leak

[Paolo Bonzini]

On 11/02/2015 17:44, Gerd Hoffmann wrote:
> Found by coverity.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  monitor.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/monitor.c b/monitor.c
> index c3cc060..29d7e5b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1101,6 +1101,7 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
>      if (strcmp(protocol, "spice") == 0) {
>          if (!qemu_using_spice(&err)) {
>              qerror_report_err(err);
> +            error_free(local_err);
>              return -1;
>          }
>  
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH] spice: fix memory leak

[Michael Tokarev]

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH] spice: fix memory leak

[Michael Tokarev]

16.02.2015 11:30, Michael Tokarev wrote:
> Applied to -trivial, thanks!

Er, nope, the right fix comes from Gonglei.

/mjt