[PATCH] 9p: local: fix memory leak

Yan Wang posted 1 patch 4 years, 4 months ago
Test asan failed
Test FreeBSD failed
Test checkpatch passed
Test docker-mingw@fedora failed
Test docker-clang@ubuntu failed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191230022347.36932-1-wangyan122@huawei.com
Maintainers: Greg Kurz <groug@kaod.org>
hw/9pfs/9p-local.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] 9p: local: fix memory leak
Posted by Yan Wang 4 years, 4 months ago
Signed-off-by: Yan Wang <wangyan122@huawei.com>
---
 hw/9pfs/9p-local.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index ca641390fb..d0592c3b45 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -947,7 +947,7 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath,
     if (ctx->export_flags & V9FS_SM_MAPPED_FILE &&
         local_is_mapped_file_metadata(ctx, name)) {
         errno = EINVAL;
-        return -1;
+        goto out;
     }
 
     odirfd = local_opendir_nofollow(ctx, odirpath);
-- 
2.19.1



Re: [PATCH] 9p: local: fix memory leak
Posted by Philippe Mathieu-Daudé 4 years, 4 months ago
Please describe the memory leak, and if possible how you noticed it.

Indeed the previous 2 calls to g_path_get_dirname() aren't free'd.

On 12/30/19 3:23 AM, Yan Wang wrote:
> Signed-off-by: Yan Wang <wangyan122@huawei.com>
> ---
>   hw/9pfs/9p-local.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index ca641390fb..d0592c3b45 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -947,7 +947,7 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath,
>       if (ctx->export_flags & V9FS_SM_MAPPED_FILE &&
>           local_is_mapped_file_metadata(ctx, name)) {
>           errno = EINVAL;
> -        return -1;
> +        goto out;

As the rest of this function... OK.
With some love this function could get clever by only allocating 
variables it will use.

With better commit description:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>       }
>   
>       odirfd = local_opendir_nofollow(ctx, odirpath);
> 


Re: [PATCH] 9p: local: fix memory leak
Posted by Christian Schoenebeck 4 years, 4 months ago
On Montag, 30. Dezember 2019 03:23:47 CET Yan Wang wrote:
> Signed-off-by: Yan Wang <wangyan122@huawei.com>
> ---
>  hw/9pfs/9p-local.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index ca641390fb..d0592c3b45 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -947,7 +947,7 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath,
> if (ctx->export_flags & V9FS_SM_MAPPED_FILE &&
>          local_is_mapped_file_metadata(ctx, name)) {
>          errno = EINVAL;
> -        return -1;
> +        goto out;
>      }
> 
>      odirfd = local_opendir_nofollow(ctx, odirpath);

This issue was already addressed 10 days ago and is scheduled for merge on 
Greg's 9p-next branch:

https://github.com/gkurz/qemu/commit/ac5987075f42e44419a461846fdefde8ab4dd624

https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg04774.html



Re: [PATCH] 9p: local: fix memory leak
Posted by Greg Kurz 4 years, 4 months ago
On Mon, 30 Dec 2019 12:34:51 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Montag, 30. Dezember 2019 03:23:47 CET Yan Wang wrote:
> > Signed-off-by: Yan Wang <wangyan122@huawei.com>
> > ---
> >  hw/9pfs/9p-local.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > index ca641390fb..d0592c3b45 100644
> > --- a/hw/9pfs/9p-local.c
> > +++ b/hw/9pfs/9p-local.c
> > @@ -947,7 +947,7 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath,
> > if (ctx->export_flags & V9FS_SM_MAPPED_FILE &&
> >          local_is_mapped_file_metadata(ctx, name)) {
> >          errno = EINVAL;
> > -        return -1;
> > +        goto out;
> >      }
> > 
> >      odirfd = local_opendir_nofollow(ctx, odirpath);
> 
> This issue was already addressed 10 days ago and is scheduled for merge on 
> Greg's 9p-next branch:
> 
> https://github.com/gkurz/qemu/commit/ac5987075f42e44419a461846fdefde8ab4dd624
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg04774.html
> 
> 

That's correct.

Philippe,

If the commit description meets your expectations, just tell me and
I'll add your R-b.

Cheers and happy new year,

--
Greg