diff mbox series

[v2] vhost-user: fix memory leak

Message ID 20180213050837.40964-1-linzhecheng@huawei.com
State New
Headers show
Series [v2] vhost-user: fix memory leak | expand

Commit Message

linzhecheng Feb. 13, 2018, 5:08 a.m. UTC
g_free() was moved from vhost_net_cleanup in commit e6bcb1b, so we should
free net after vhost_net_cleanup

Signed-off-by: linzhecheng <linzhecheng@huawei.com>

Comments

Michael S. Tsirkin Feb. 13, 2018, 4:27 p.m. UTC | #1
On Tue, Feb 13, 2018 at 01:08:37PM +0800, linzhecheng wrote:
> g_free() was moved from vhost_net_cleanup in commit e6bcb1b, so we should
> free net after vhost_net_cleanup
> 
> Signed-off-by: linzhecheng <linzhecheng@huawei.com>

Looks like this needs a Fixes: tag and CC stable - is that right?

> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index cb45512506..d024573e45 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -109,6 +109,7 @@ static int vhost_user_start(int queues, NetClientState *ncs[], CharBackend *be)
>  err:
>      if (net) {
>          vhost_net_cleanup(net);
> +        g_free(net);
>      }
>      vhost_user_stop(i, ncs);
>      return -1;
> -- 
> 2.12.2.windows.2
>
Marc-André Lureau Feb. 13, 2018, 5:01 p.m. UTC | #2
On Tue, Feb 13, 2018 at 6:08 AM, linzhecheng <linzhecheng@huawei.com> wrote:
> g_free() was moved from vhost_net_cleanup in commit e6bcb1b, so we should
> free net after vhost_net_cleanup
>
> Signed-off-by: linzhecheng <linzhecheng@huawei.com>

This can be reproduced and catched by ASAN too:

==11833==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 784 byte(s) in 1 object(s) allocated from:
    #0 0x7fa1aed89a38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
    #1 0x7fa1adadff75 in g_malloc0 ../glib/gmem.c:124
    #2 0x56082a195515 in vhost_net_init
/home/elmarco/src/qemu/hw/net/vhost_net.c:147
    #3 0x61500000bbff  (<unknown module>)


good catch,

Reviewed-by: Marc-André Lureau < marcandre.lureau@redhat.com>

>
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index cb45512506..d024573e45 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -109,6 +109,7 @@ static int vhost_user_start(int queues, NetClientState *ncs[], CharBackend *be)
>  err:
>      if (net) {
>          vhost_net_cleanup(net);
> +        g_free(net);
>      }
>      vhost_user_stop(i, ncs);
>      return -1;
> --
> 2.12.2.windows.2
>
>
>
Philippe Mathieu-Daudé Feb. 13, 2018, 8:37 p.m. UTC | #3
On 02/13/2018 02:08 AM, linzhecheng wrote:
> g_free() was moved from vhost_net_cleanup in commit e6bcb1b, so we should
> free net after vhost_net_cleanup
> 
> Signed-off-by: linzhecheng <linzhecheng@huawei.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index cb45512506..d024573e45 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -109,6 +109,7 @@ static int vhost_user_start(int queues, NetClientState *ncs[], CharBackend *be)
>  err:
>      if (net) {
>          vhost_net_cleanup(net);
> +        g_free(net);
>      }
>      vhost_user_stop(i, ncs);
>      return -1;
>
diff mbox series

Patch

diff --git a/net/vhost-user.c b/net/vhost-user.c
index cb45512506..d024573e45 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -109,6 +109,7 @@  static int vhost_user_start(int queues, NetClientState *ncs[], CharBackend *be)
 err:
     if (net) {
         vhost_net_cleanup(net);
+        g_free(net);
     }
     vhost_user_stop(i, ncs);
     return -1;