[v1] virtiofs daemon [all]

[PATCH 079/104] virtiofsd: fix memory leak on lo.source

Posted by Dr. David Alan Gilbert (git) 6 years ago

From: Liu Bo <bo.liu@linux.alibaba.com>

valgrind reported that lo.source is leaked on quiting, but it was defined
as (const char*) as it may point to a const string "/".

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
 tools/virtiofsd/passthrough_ll.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 33092de65a..45cf466178 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2529,9 +2529,8 @@ int main(int argc, char *argv[])
             fuse_log(FUSE_LOG_ERR, "source is not a directory\n");
             exit(1);
         }
-
     } else {
-        lo.source = "/";
+        lo.source = strdup("/");
     }
     lo.root.is_symlink = false;
     if (!lo.timeout_set) {
@@ -2610,5 +2609,7 @@ err_out1:
         close(lo.root.fd);
     }
 
+    free((char *)lo.source);
+
     return ret ? 1 : 0;
 }
-- 
2.23.0

Re: [PATCH 079/104] virtiofsd: fix memory leak on lo.source

Posted by Daniel P. Berrangé 5 years, 11 months ago

On Thu, Dec 12, 2019 at 04:38:39PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: Liu Bo <bo.liu@linux.alibaba.com>
> 
> valgrind reported that lo.source is leaked on quiting, but it was defined
> as (const char*) as it may point to a const string "/".
> 
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> ---
>  tools/virtiofsd/passthrough_ll.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index 33092de65a..45cf466178 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2529,9 +2529,8 @@ int main(int argc, char *argv[])
>              fuse_log(FUSE_LOG_ERR, "source is not a directory\n");
>              exit(1);
>          }
> -
>      } else {
> -        lo.source = "/";
> +        lo.source = strdup("/");
>      }
>      lo.root.is_symlink = false;
>      if (!lo.timeout_set) {
> @@ -2610,5 +2609,7 @@ err_out1:
>          close(lo.root.fd);
>      }
>  
> +    free((char *)lo.source);

Can we not change the 'lo_data' struct so that source is not const
and thus avoid free'ing a const field ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 079/104] virtiofsd: fix memory leak on lo.source

Posted by Dr. David Alan Gilbert 5 years, 11 months ago

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Thu, Dec 12, 2019 at 04:38:39PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: Liu Bo <bo.liu@linux.alibaba.com>
> > 
> > valgrind reported that lo.source is leaked on quiting, but it was defined
> > as (const char*) as it may point to a const string "/".
> > 
> > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> > ---
> >  tools/virtiofsd/passthrough_ll.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > index 33092de65a..45cf466178 100644
> > --- a/tools/virtiofsd/passthrough_ll.c
> > +++ b/tools/virtiofsd/passthrough_ll.c
> > @@ -2529,9 +2529,8 @@ int main(int argc, char *argv[])
> >              fuse_log(FUSE_LOG_ERR, "source is not a directory\n");
> >              exit(1);
> >          }
> > -
> >      } else {
> > -        lo.source = "/";
> > +        lo.source = strdup("/");
> >      }
> >      lo.root.is_symlink = false;
> >      if (!lo.timeout_set) {
> > @@ -2610,5 +2609,7 @@ err_out1:
> >          close(lo.root.fd);
> >      }
> >  
> > +    free((char *)lo.source);
> 
> Can we not change the 'lo_data' struct so that source is not const
> and thus avoid free'ing a const field ?

Done.  Made that free(lo.source) and dropped the const.

Dave


> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK