Re: libfuse: patch to prevent fuse-zip from dumping core

[Helg Bredow]

On Wed, 18 Oct 2017 10:04:07 +0200
Martin Pieuchot <m...@openbsd.org> wrote:

> On 17/10/17(Tue) 15:30, Helg Bredow wrote:
> > If you execute "fuse-zip -V" it prints the version and then dumps core. 
> > This is because fuse-zip does not initialise the mount point pointer to 
> > NULL. This patch ensures that it's always initialised to NULL.
> 
> It's hard to understand your fix if you don't explain what "dumps core".
> 
> I had to install the package and look at the trace myself.  You could
> save me these tasks by either posting the backtrace, saying that free(3)
> is call on an uninitialized memory or both.
> 
> That said, I'd suggest different fix.  Initializing `mp' in fuse_setup()
> is very fragile.  Instead I'd declare a local variable and don't use
> `mp' at all in these function.
> In case of sucsses, just before returning the "struct fuse" pointer I'd
> assign *mp, if not NULL, to the local variable.
> 
> By the way, what does "mp" stand for?  I find the name confusing.
> 

Sorry, I've been looking at this for so long I assumed the diff was 
self-explanatory. Thanks for your patience. I like your suggestion and have 
modified the patch accordingly. A NULL test before the assignment is 
superfluous since the code won't be reached if dir is NULL.

`mp' is the directory where the fuse file system will be mounted (mount point). 
I've named the new variable dir to be consistent with mount(2).

Index: fuse.c
===================================================================
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.29
diff -u -p -r1.29 fuse.c
--- fuse.c      21 Aug 2017 21:41:13 -0000      1.29
+++ fuse.c      18 Oct 2017 12:39:12 -0000
@@ -466,14 +466,16 @@ fuse_setup(int argc, char **argv, const 
        struct fuse_args args = FUSE_ARGS_INIT(argc, argv);
        struct fuse_chan *fc;
        struct fuse *fuse;
+       char *dir;
        int fg;
 
-       if (fuse_parse_cmdline(&args, mp, mt, &fg))
+       dir = NULL;
+       if (fuse_parse_cmdline(&args, &dir, mt, &fg))
                goto err;
 
        fuse_daemonize(0);
 
-       if ((fc = fuse_mount(*mp, NULL)) == NULL)
+       if ((fc = fuse_mount(dir, NULL)) == NULL)
                goto err;
 
        if ((fuse = fuse_new(fc, NULL, ops, size, data)) == NULL) {
@@ -481,9 +483,10 @@ fuse_setup(int argc, char **argv, const 
                goto err;
        }
 
+       *mp = dir;
        return (fuse);
 err:
-       free(*mp);
+       free(dir);
        return (NULL);
 }