From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:35907)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <groug@kaod.org>) id 1c5YzA-0004SW-O5
	for qemu-devel@nongnu.org; Sat, 12 Nov 2016 09:08:49 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <groug@kaod.org>) id 1c5Yz6-0001GX-O9
	for qemu-devel@nongnu.org; Sat, 12 Nov 2016 09:08:48 -0500
Received: from 3.mo53.mail-out.ovh.net ([178.33.44.239]:49278)
	by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)
	(Exim 4.71) (envelope-from <groug@kaod.org>) id 1c5Yz6-0001GJ-Hu
	for qemu-devel@nongnu.org; Sat, 12 Nov 2016 09:08:44 -0500
Received: from player158.ha.ovh.net (b7.ovh.net [213.186.33.57])
	by mo53.mail-out.ovh.net (Postfix) with ESMTP id 1B1B241A2C
	for <qemu-devel@nongnu.org>; Sat, 12 Nov 2016 15:08:42 +0100 (CET)
Date: Sat, 12 Nov 2016 15:08:36 +0100
From: Greg Kurz <groug@kaod.org>
Message-ID: <20161112150836.24a07b75@bahia>
In-Reply-To: <5826cf4d.4b866b0a.caff8.66a8@mx.google.com>
References: <5826cf4d.4b866b0a.caff8.66a8@mx.google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] 9pfs: free the private data memory in
 device unrealize
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Li Qiang <liq3ea@gmail.com>
Cc: qemu-devel@nongnu.org, Li Qiang <liqiang6-s@360.cn>

Hi Li,

On Sat, 12 Nov 2016 00:13:56 -0800
Li Qiang <liq3ea@gmail.com> wrote:

> From: Li Qiang <liqiang6-s@360.cn>
> 
> When the 9pfs use 'handle'/'proxy' as the backend driver it
> will allocate a private data memory. But this memory is not freed in
> the unrealize function thus leading a memory leak issue. This patch
> avoid this.
> 
> Signed-off-by: Li Qiang <liqiang6-s@360.cn>
> ---
>  hw/9pfs/9p.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index aea7e9d..6524b13 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3532,6 +3532,7 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
>  {
>      g_free(s->ctx.fs_root);
>      g_free(s->tag);
> +    g_free(s->ctx.private);

This is wrong: we cannot do any assumptions on the private data in the core
code. It can only be used in the backend code. You need to add a .cleanup
operation to the FileOperations structure and implement it in the handle and
proxy backends.

Also, it appears that there's more to be done than just freeing the private
data itself:
- handle needs to close mountfd
- proxy needs to close sockfd and free some allocated iov_base

Cheers.

--
Greg

>  }
>  
>  typedef struct VirtfsCoResetData {