Read/Write whole fusebufs

[Helg]

Hello tech@

Due to the challenges of having a large diff reviewed I've had another
think about how I can break up the FUSE changes so that they are smaller
and easier to review.

This is the first of these diffs.

The current design uses a fixed size fusebuf that consists of a header
and a union of structs that are used for different VFS operations. In
addition, there may be data of variable size associated with an
operation. e.g. the buffer passed to write(2). see fb_setup(9).

If there is additional data to be exchanged between libfuse and the
kernel then libfuse uses an ioctl on the device to read or write this
variable sized data after the fusebuf has been read or written.  This is
not how the fuse protocol works on Linux.  Instead, the fusebuf is read
or written in a single read(2) or write(2).  This change is the first
step in setting the OpenBSD implementation up for improved compatibility
in the fuse kernel interface.

The fusebuf struct is shared between the kernel and libfuse but its
layout differs slightly between the two. The kernel has knowledge of the
size of data that it is sending or receiving (e.g. read, write, readdir,
link, lookup) and so can malloc the exact amount of memory required.
libfuse must read the entire fusebuf but doesn't know its size in
advance so must have a buffer large enough to cater for the worst case
scenario. Since libfuse now uses a fixed size fusebuf, it no longer
needs to free the variable memory previously allocated for the data.

stsp@ has been kind enough to provide initial feedback. Is it now ready
for an official OK?



Index: lib/libfuse/fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.51
diff -u -p -r1.51 fuse.c
--- lib/libfuse/fuse.c  28 Jun 2019 13:32:42 -  1.51
+++ lib/libfuse/fuse.c  8 Jun 2021 14:15:29 -
@@ -154,9 +154,9 @@ fuse_loop(struct fuse *fuse)
 {
struct fusebuf fbuf;
struct fuse_context ctx;
-   struct fb_ioctl_xch ioexch;
struct kevent event[5];
struct kevent ev;
+   ssize_t fbuf_size;
ssize_t n;
int ret;
 
@@ -201,29 +201,15 @@ fuse_loop(struct fuse *fuse)
strsignal(signum));
}
} else if (ret > 0) {
-   n = read(fuse->fc->fd, , sizeof(fbuf));
-   if (n != sizeof(fbuf)) {
+   n = read(fuse->fc->fd, , FUSEBUFSIZE);
+   fbuf_size = sizeof(fbuf.fb_hdr) + sizeof(fbuf.FD) +
+   fbuf.fb_len;
+   if (n != fbuf_size) {
fprintf(stderr, "%s: bad fusebuf read\n",
__func__);
return (-1);
}
 
-   /* check if there is data something present */
-   if (fbuf.fb_len) {
-   fbuf.fb_dat = malloc(fbuf.fb_len);
-   if (fbuf.fb_dat == NULL)
-   return (-1);
-   ioexch.fbxch_uuid = fbuf.fb_uuid;
-   ioexch.fbxch_len = fbuf.fb_len;
-   ioexch.fbxch_data = fbuf.fb_dat;
-
-   if (ioctl(fuse->fc->fd, FIOCGETFBDAT,
-   ) == -1) {
-   free(fbuf.fb_dat);
-   return (-1);
-   }
-   }
-
ctx.fuse = fuse;
ctx.uid = fbuf.fb_uid;
ctx.gid = fbuf.fb_gid;
@@ -238,26 +224,13 @@ fuse_loop(struct fuse *fuse)
return (-1);
}
 
-   n = write(fuse->fc->fd, , sizeof(fbuf));
-   if (fbuf.fb_len) {
-   if (fbuf.fb_dat == NULL) {
-   fprintf(stderr, "%s: fb_dat is Null\n",
-   __func__);
-   return (-1);
-   }
-   ioexch.fbxch_uuid = fbuf.fb_uuid;
-   ioexch.fbxch_len = fbuf.fb_len;
-   ioexch.fbxch_data = fbuf.fb_dat;
-
-   if (ioctl(fuse->fc->fd, FIOCSETFBDAT, ) 
== -1) {
-   free(fbuf.fb_dat);
-   return (-1);
-   }
-   free(fbuf.fb_dat);
-   }
+   fbuf_size = sizeof(fbuf.fb_hdr) + sizeof(fbuf.FD) +
+   fbuf.fb_len;
+   n = write(fuse->fc->fd, , fbuf_size);
+
  

Re: Replace libfuse from base with libfuse from ports

[Helg]

On Fri, Feb 05, 2021 at 07:17:09PM +0100, Gr??goire Jadi wrote:
> Helg  writes:
> 
> Hi,
> 
> > I currently only have a port of the 2.x branch of libfuse (attached) but
> > will also port the 3.x libfuse once I have user mounting working. I am
> > not aware of any ports other than the latest version of sshfs that
> > depends on 3.x. In any case, we will need to support libfuse 2.x for
> > some time until all file systems are updated upstream to work with 3.x. 
> >  
> 
> I can't comment for the rest of your work, but borgbackup seems to
> prefer libfuse 3.x

Thanks for letting me know. borgbackup is an example of a port that will
benefit from this work. Are you suggesting that I should not commit
until libfuse3 has also been ported? Even thought it's not actively
maintained, llfuse could be ported in the meantime to enable FUSE
functionality in borgbackup..

> https://github.com/borgbackup/borg/blob/master/setup.py#L77
> > # note for package maintainers: if you package borgbackup for distribution,
> > # please (if available) add pyfuse3 (preferably) or llfuse (not maintained 
> > any more)
> > # as a *requirement*. "borg mount" needs one of them to work.
> > # if neither is available, do not require it, most of borgbackup will
> > work.
> 
> https://pypi.org/project/pyfuse3/
> > pyfuse3 is a set of Python 3 bindings for libfuse 3. It provides an
> > asynchronous API compatible with Trio and asyncio, and enables you to
> > easily write a full-featured Linux filesystem in Python.
> 
> Thanks for your work.
> Best,
> 
> -- 
> gjadi
> PGP : AF26 E9C2 A1C8 8D32 A868  4386 1373 5477 2B65 1894

Re: fuse: vfs create does not map 1:1 to fuse create

[Helg]

On Mon, Nov 27, 2017 at 10:48:04AM +0100, Martin Pieuchot wrote:
> On 23/11/17(Thu) 21:45, Helg wrote:
> > On Thu, Nov 23, 2017 at 12:09:34PM +0000, Helg Bredow wrote:
> > > - Forwarded message from Martin Pieuchot <m...@openbsd.org> -
> > > 
> > > Date: Sat, 18 Nov 2017 11:03:49 +0100
> > > From: Martin Pieuchot <m...@openbsd.org>
> > > To: Helg Bredow <xx...@msn.com>
> > > CC: "tech@openbsd.org" <tech@openbsd.org>
> > > Subject: Re: fuse: vfs create does not map 1:1 to fuse create
> > > User-Agent: Mutt/1.9.1 (2017-09-22)
> > > 
> > > On 18/11/17(Sat) 02:22, Helg Bredow wrote:
> > > > On Fri, 10 Nov 2017 09:09:32 +0100
> > > > Martin Pieuchot <m...@openbsd.org> wrote:
> > > > 
> > > > > On 09/11/17(Thu) 01:20, Helg Bredow wrote:
> > > > > > > On 08/11/17(Wed) 14:12, Helg Bredow wrote:
> > > > > > > > There is a bug when creating a file in fuse-exfat and then 
> > > > > > > > deleting it
> > > > > > > > again without first unmounting the file system. The reason for 
> > > > > > > > this is
> > > > > > > > that fuse-exfat maintains strict reference counts and fuse 
> > > > > > > > currently
> > > > > > > > calls the file system create and open functions when it should 
> > > > > > > > only
> > > > > > > > call create. 
> > > > > > > > [...]
> > > > > > > 
> > > > [...]
> > > > > 
> > [...]
> > > > 
> > > > I now this caused some confusion but I believe the patch below is the
> > > > correct solution.
> > > 
> > > When you use the work "believing" you're asking us to trust you without
> > > argumenting.
> > > 
> > > >   Here's the mapping of file system calls so it's clear.
> > > > [...]
> > > 
> > > That's irrelevant.
> > > 
> > > There's no such thing as "atomic_open" from the userland point of view.
> > > So what you're discussing here is an "implementation details" of Linux
> > > VFS.
> > > 
> > > What matters is which syscall the application do.  I believe it is
> > > open(2).  So how its arguments are translated to the userland FS?  If
> > > I understand your diff correctly, OpenBSD will now do something like
> > > below (please correct the graph):
> > > 
> > > 
> > >   USER PROCESSfuse-zip
> > >   |   ^
> > >open(2)| 
> > >   |   FBT_MKNOD + FBT_OPEN 
> > >  \/ |
> > >   -
> > >   
> > > KERNEL
> > > 
> > > The only thing I understand is that you're changing the kernel behavior
> > > for all open(2) syscall operating on FUSE filesystem.  Before the kernel
> > > was generating a FBT_CREATE and FBT_OPEN messages, with some arguments.
> > > Now you want to generate FTB_MKNOD and FBT_OPEN with other arguments.
> > > 
> > > To me it sounds that you're working around a problem you don't
> > > understand.  If FUSE FSs want a FUSE 'create' operation then let's
> > > give them that!  What argument do they expect?
> > > 
> > > So let's start from the beginning:
> > > 
> > > - Which syscall is causing problem with which arguments?
> > > - Which FUSE operation(s) FUSE FSs are expecting for this exact syscall
> > >   with these arguments? 
> > > - Which FUSE operation(s) OpenBSD FUSE is currently generating with which
> > >   arguments?
> >  
> > The syscall that is causing the problem is open(2) with the O_CREAT flag.
> > With my change, this is the call graph.
> >  
> >  open(2)
> >|
> >  vn_open()
> >|
> >+--VOP_CREATE(9) when O_CREAT
> >|   |
> >|   +-->fusefs_create()-->FBT_MKNOD-->ifuse_ops_mknod()
> >|   |
> >|   +--> fs mknod()
> >|
> >+--VOP_SETATTR(9) when O_TRUNC
> >|   |
> >|   +-->fusefs_setattr()-->FBT_SETATTR-->ifuse_ops_setattr()
> >|   |
> >|

FUSE: link should return EPERM if the source file is a directory

[Helg]

fusefs_link returns the wrong error code when attempting to create a
hard link to a directory. It returns EISDIR when it should instead
return EPERM.  Discovered while running the ffs test suite on ntfs-3g
and confirmed by comparing to ufs.

This is the description for the test that fails:


"link returns EPERM if the source file is a directory"

This patch changes the error code to EPERM.

ok?

Index: fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.35
diff -u -p -u -p -r1.35 fuse_vnops.c
--- fuse_vnops.c27 Nov 2017 12:54:13 -  1.35
+++ fuse_vnops.c27 Nov 2017 13:49:15 -
@@ -565,7 +565,7 @@ fusefs_link(void *v)
}
if (vp->v_type == VDIR) {
VOP_ABORTOP(dvp, cnp);
-   error = EISDIR;
+   error = EPERM;
goto out2;
}
if (dvp->v_mount != vp->v_mount) {

FUSE: fuse_lookup doesn't check access on create

[Helg]

Hi tech@

It should not be possible to create a file or directory if write
permission is denied on the parent directory of the file or directory to
be created. However, FUSE does not perform an access check when the new
vnode lookup is performed and always allows files and directory to be
created unless the file system is mounted read-only.

This patch adds the access check to fuse_lookup.
(copied from ufs_lookup.c)

ok?


Index: fuse_lookup.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_lookup.c,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 fuse_lookup.c
--- fuse_lookup.c   7 Sep 2016 17:53:35 -   1.16
+++ fuse_lookup.c   26 Nov 2017 11:18:49 -
@@ -93,6 +93,14 @@ fusefs_lookup(void *v)
if (vdp->v_mount->mnt_flag & MNT_RDONLY)
return (EROFS);
 
+   /*
+* Access for write is interpreted as allowing
+* creation of files in the directory.
+*/
+   if ((error = VOP_ACCESS(vdp, VWRITE, cred,
+   cnp->cn_proc)) != 0)
+   return (error); 
+
cnp->cn_flags |= SAVENAME;
 
if (!lockparent) {

libfuse: support -f option

[Helg]

This patch adds support for the -f option, which forces the FUSE file
system to run in the foreground. This is useful for debugging and adds
a missing feature to the library.

ok?


Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.35
diff -u -p -u -p -r1.35 fuse.c
--- fuse.c  17 Nov 2017 15:56:12 -  1.35
+++ fuse.c  26 Nov 2017 08:27:39 -
@@ -37,6 +37,7 @@ static struct fuse_context *ictx = NULL;
 static int max_read = FUSEBUFMAXSIZE;
 
 enum {
+   KEY_FOREGROUND,
KEY_HELP,
KEY_HELP_WITHOUT_HEADER,
KEY_VERSION,
@@ -53,7 +54,7 @@ static struct fuse_opt fuse_core_opts[] 
FUSE_OPT_KEY("max_read=",   KEY_MAXREAD),
FUSE_OPT_KEY("debug",   KEY_STUB),
FUSE_OPT_KEY("-d",  KEY_STUB),
-   FUSE_OPT_KEY("-f",  KEY_STUB),
+   FUSE_OPT_KEY("-f",  KEY_FOREGROUND),
FUSE_OPT_KEY("-s",  KEY_STUB),
FUSE_OPT_KEY("use_ino", KEY_STUB),
FUSE_OPT_KEY("big_writes",  KEY_STUB),
@@ -342,13 +343,12 @@ fuse_new(struct fuse_chan *fc, unused st
 }
 
 int
-fuse_daemonize(unused int foreground)
+fuse_daemonize(int foreground)
 {
-#ifdef DEBUG
-   return (daemon(0,1));
-#else
+   if (foreground)
+   return (0);
+
return (daemon(0,0));
-#endif
 }
 
 void
@@ -387,6 +387,7 @@ dump_help(void)
 {
fprintf(stderr, "FUSE options:\n"
"-d   -o debug  enable debug output (implies -f)\n"
+   "-f run in foreground\n"
"-V print fuse version\n"
"\n");
 }
@@ -409,6 +410,9 @@ ifuse_process_opt(void *data, const char
switch (key) {
case KEY_STUB:
return (0);
+   case KEY_FOREGROUND:
+   opt->foreground = 1;
+   return (0);
case KEY_HELP:
case KEY_HELP_WITHOUT_HEADER:
dump_help();
@@ -460,7 +464,7 @@ ifuse_process_opt(void *data, const char
 }
 
 int
-fuse_parse_cmdline(struct fuse_args *args, char **mp, int *mt, unused int *fg)
+fuse_parse_cmdline(struct fuse_args *args, char **mp, int *mt, int *fg)
 {
struct fuse_core_opt opt;
 
@@ -485,6 +489,9 @@ fuse_parse_cmdline(struct fuse_args *arg
if (mt != NULL)
*mt = 0;
 
+   if (fg != NULL)
+   *fg = opt.foreground;
+
return (0);
 }
 
@@ -530,7 +537,7 @@ fuse_setup(int argc, char **argv, const 
if (fuse_parse_cmdline(, , mt, ))
goto err;
 
-   fuse_daemonize(0);
+   fuse_daemonize(fg);
 
if ((fc = fuse_mount(dir, NULL)) == NULL)
goto err;
Index: fuse_private.h
===
RCS file: /cvs/src/lib/libfuse/fuse_private.h,v
retrieving revision 1.14
diff -u -p -u -p -r1.14 fuse_private.h
--- fuse_private.h  7 Sep 2016 17:53:35 -   1.14
+++ fuse_private.h  26 Nov 2017 08:27:39 -
@@ -74,7 +74,8 @@ struct fuse_config {
 };
 
 struct fuse_core_opt {
-   char *mp;
+   char*mp;
+   int foreground;
 };
 
 struct fuse {

FUSE: rename doesn't unlock target vnode

[Helg]

When renaming a file on a FUSE file system and the target exists and is
open, a deadlock will occur because the target vnode (tvp) has not been
unlocked by fusefs_rename(). When the process that has the file open
calls close(2) the vnode will still be locked with no chance of ever
being unlocked.

ok?


Index: fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.34
diff -u -p -u -p -r1.34 fuse_vnops.c
--- fuse_vnops.c17 Nov 2017 15:45:17 -  1.34
+++ fuse_vnops.c24 Nov 2017 23:20:14 -
@@ -1261,6 +1261,8 @@ abortit:
vrele(tdvp);
else
vput(tdvp);
+   if (tvp)
+   vput(tvp);
vrele(fdvp);
vrele(fvp);
 

VOP_LOOKUP(9) man page VOP_RENAME(9) errata

[Helg]

The VOP_LOOKUP(9) man page incorrectly states the following about
VOP_RENAME(9).

"If not NULL, tvp will be locked on return as well."

However, dorenameat() in /sys/kern/vfs_syscalls.c unlocks tvp on error
and file systems unlock it on success. This patch changes this line to
read "unlocked".

ok?


Index: VOP_LOOKUP.9
===
RCS file: /cvs/src/share/man/man9/VOP_LOOKUP.9,v
retrieving revision 1.38
diff -u -p -p -u -r1.38 VOP_LOOKUP.9
--- VOP_LOOKUP.926 Apr 2017 05:55:27 -  1.38
+++ VOP_LOOKUP.924 Nov 2017 22:54:24 -
@@ -855,7 +855,7 @@ on return.
 If not
 .Dv NULL ,
 .Fa tvp
-will be locked on return as well.
+will be unlocked on return as well.
 Upon success, zero is returned; otherwise, an appropriate error code is
 returned.
 .Pp

Re: fuse: vfs create does not map 1:1 to fuse create

[Helg]

On Thu, Nov 23, 2017 at 12:09:34PM +, Helg Bredow wrote:
> - Forwarded message from Martin Pieuchot <m...@openbsd.org> -
> 
> Date: Sat, 18 Nov 2017 11:03:49 +0100
> From: Martin Pieuchot <m...@openbsd.org>
> To: Helg Bredow <xx...@msn.com>
> CC: "tech@openbsd.org" <tech@openbsd.org>
> Subject: Re: fuse: vfs create does not map 1:1 to fuse create
> User-Agent: Mutt/1.9.1 (2017-09-22)
> 
> On 18/11/17(Sat) 02:22, Helg Bredow wrote:
> > On Fri, 10 Nov 2017 09:09:32 +0100
> > Martin Pieuchot <m...@openbsd.org> wrote:
> > 
> > > On 09/11/17(Thu) 01:20, Helg Bredow wrote:
> > > > > On 08/11/17(Wed) 14:12, Helg Bredow wrote:
> > > > > > There is a bug when creating a file in fuse-exfat and then deleting 
> > > > > > it
> > > > > > again without first unmounting the file system. The reason for this 
> > > > > > is
> > > > > > that fuse-exfat maintains strict reference counts and fuse currently
> > > > > > calls the file system create and open functions when it should only
> > > > > > call create. 
> > > > > > [...]
> > > > > 
> > [...]
> > > 
[...]
> > 
> > I now this caused some confusion but I believe the patch below is the
> > correct solution.
> 
> When you use the work "believing" you're asking us to trust you without
> argumenting.
> 
> >   Here's the mapping of file system calls so it's clear.
> > [...]
> 
> That's irrelevant.
> 
> There's no such thing as "atomic_open" from the userland point of view.
> So what you're discussing here is an "implementation details" of Linux
> VFS.
> 
> What matters is which syscall the application do.  I believe it is
> open(2).  So how its arguments are translated to the userland FS?  If
> I understand your diff correctly, OpenBSD will now do something like
> below (please correct the graph):
> 
> 
>   USER PROCESSfuse-zip
>   |   ^
>open(2)| 
>   |   FBT_MKNOD + FBT_OPEN 
>  \/ |
>   -
>   
> KERNEL
> 
> The only thing I understand is that you're changing the kernel behavior
> for all open(2) syscall operating on FUSE filesystem.  Before the kernel
> was generating a FBT_CREATE and FBT_OPEN messages, with some arguments.
> Now you want to generate FTB_MKNOD and FBT_OPEN with other arguments.
> 
> To me it sounds that you're working around a problem you don't
> understand.  If FUSE FSs want a FUSE 'create' operation then let's
> give them that!  What argument do they expect?
> 
> So let's start from the beginning:
> 
> - Which syscall is causing problem with which arguments?
> - Which FUSE operation(s) FUSE FSs are expecting for this exact syscall
>   with these arguments? 
> - Which FUSE operation(s) OpenBSD FUSE is currently generating with which
>   arguments?
 
The syscall that is causing the problem is open(2) with the O_CREAT flag.
With my change, this is the call graph.
 
 open(2)
   |
 vn_open()
   |
   +--VOP_CREATE(9) when O_CREAT
   |   |
   |   +-->fusefs_create()-->FBT_MKNOD-->ifuse_ops_mknod()
   |   |
   |   +--> fs mknod()
   |
   +--VOP_SETATTR(9) when O_TRUNC
   |   |
   |   +-->fusefs_setattr()-->FBT_SETATTR-->ifuse_ops_setattr()
   |   |
   |   +--> fs truncate()
   |
   +--VOP_OPEN(9)   always
   |
   +--fusefs_open()-->FBT_OPEN-->ifuse_ops_open()
   |
   +--> fs open()
 
 
The file systems are expecting the following arguments.

mknod(path, mode, dev)
   mode is that passed to open(2)
   dev will be 0 for regular files. FUSE mknod can create regular files.
open(path, fuse_file_info)
   fuse_file_info has (almost all) flags passed to open(2) plus other
   things that are not set by ifuse_ops_open. The FS will also return
   a handle to the file that was opened which it expects to receive
   again for functions that operate on the same file.
create(path, mode, fuse_file_info)
   mode is that passed to open(2)
   fuse_file_info has (almost all) flags passed to open(2) plus other
   things that are not set by ifuse_ops_open. The FS will also return
   a handle to the file that was opened which it expects to receive
   again for functions that operate on the same file.
   
Open

Re: fuse: vfs create does not map 1:1 to fuse create

[Helg Bredow]

On Fri, 10 Nov 2017 09:09:32 +0100
Martin Pieuchot <m...@openbsd.org> wrote:

> On 09/11/17(Thu) 01:20, Helg Bredow wrote:
> > > On 08/11/17(Wed) 14:12, Helg Bredow wrote:
> > > > There is a bug when creating a file in fuse-exfat and then deleting it
> > > > again without first unmounting the file system. The reason for this is
> > > > that fuse-exfat maintains strict reference counts and fuse currently
> > > > calls the file system create and open functions when it should only
> > > > call create. 
> > > > [...]
> > > 
[...]
> 
> In the end that's your call.  Nobody understand the problem, what you
> say is "we are not linux".  Which we already know.  And you're pushing
> for a diff which works saying that the best solution.  It believe it
> fixes the problem so go for it.

I now this caused some confusion but I believe the patch below is the
correct solution. Here's the mapping of file system calls so it's clear.

vfs | fuse
+
atomic_open | create
create  | mknod
mknod   | mknod
open| open

encfs and fuse-exfat now work.
curlftsfs now returns an error on file append since it doesn't support
it.

I've contacted the developer of fuse-zip and he's adding mknod()
support.

ok?


Index: sys/miscfs/fuse/fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.33
diff -u -p -p -u -r1.33 fuse_vnops.c
--- sys/miscfs/fuse/fuse_vnops.c7 Sep 2016 17:53:35 -   1.33
+++ sys/miscfs/fuse/fuse_vnops.c18 Nov 2017 01:58:34 -
@@ -211,7 +211,7 @@ fusefs_open(void *v)
struct fusefs_node *ip;
struct fusefs_mnt *fmp;
enum fufh_type fufh_type = FUFH_RDONLY;
-   int flags = O_RDONLY;
+   int flags;
int error;
int isdir;
 
@@ -226,24 +226,27 @@ fusefs_open(void *v)
if (ap->a_vp->v_type == VDIR)
isdir = 1;
else {
-   if ((ap->a_mode & FREAD) && (ap->a_mode & FWRITE)) {
+   if ((ap->a_mode & FREAD) && (ap->a_mode & FWRITE))
fufh_type = FUFH_RDWR;
-   flags = O_RDWR;
-   } else if (ap->a_mode  & (FWRITE)) {
+   else if (ap->a_mode  & (FWRITE))
fufh_type = FUFH_WRONLY;
-   flags = O_WRONLY;
-   }
}
 
/* already open i think all is ok */
if (ip->fufh[fufh_type].fh_type != FUFH_INVALID)
return (0);
 
+   /* no creation and truncation flags are passed to open */
+   flags = OFLAGS(ap->a_mode);
+   flags &= ~O_CREAT;
+   flags &= ~O_EXCL;
+   flags &= ~O_TRUNC;
+
error = fusefs_file_open(fmp, ip, fufh_type, flags, isdir, ap->a_p);
if (error)
return (error);
 
-   return (error);
+   return (0);
 }
 
 int
@@ -891,16 +894,15 @@ fusefs_create(void *v)
goto out;
}
 
-   if (fmp->undef_op & UNDEF_CREATE) {
+   if (fmp->undef_op & UNDEF_MKNOD) {
error = ENOSYS;
goto out;
}
 
fbuf = fb_setup(cnp->cn_namelen + 1, ip->ufs_ino.i_number,
-   FBT_CREATE, p);
+   FBT_MKNOD, p);
 
fbuf->fb_io_mode = mode;
-   fbuf->fb_io_flags = O_CREAT | O_RDWR;
 
memcpy(fbuf->fb_dat, cnp->cn_nameptr, cnp->cn_namelen);
fbuf->fb_dat[cnp->cn_namelen] = '\0';
@@ -908,7 +910,7 @@ fusefs_create(void *v)
error = fb_queue(fmp->dev, fbuf);
if (error) {
if (error == ENOSYS)
-   fmp->undef_op |= UNDEF_CREATE;
+   fmp->undef_op |= UNDEF_MKNOD;
 
fb_delete(fbuf);
goto out;
Index: lib/libfuse/fuse_ops.c
===
RCS file: /cvs/src/lib/libfuse/fuse_ops.c,v
retrieving revision 1.27
diff -u -p -p -u -r1.27 fuse_ops.c
--- lib/libfuse/fuse_ops.c  17 Nov 2017 15:45:17 -  1.27
+++ lib/libfuse/fuse_ops.c  18 Nov 2017 01:58:34 -
@@ -579,6 +579,8 @@ ifuse_ops_write(struct fuse *f, struct f
return (0);
 }
 
+#if 0
+/* fuse create requires the kernel to support atomic_open() */
 static int
 ifuse_ops_create(struct fuse *f, struct fusebuf *fbuf)
 {
@@ -611,8 +613,6 @@ ifuse_ops_create(struct fuse *f, struct 
 
if (f->op.create)
fbuf->fb_err = f->op.create(realname, mode,  );
-   else if (f->op.mknod)
-   fbuf->fb_err = f->op.mknod(realname, S_IFREG | mode, 0);
else
fbuf->fb_err = -ENOSYS;
 
@@ -625,6 +625,7 @@ ifuse_ops_create(struct fuse *f, struct 
 

Re: libfuse: signal handler doesn't cater for "Device busy" and other errors

[Helg Bredow]

On Fri, 10 Nov 2017 11:55:53 +
Helg Bredow <xx...@msn.com> wrote:

> On Fri, 10 Nov 2017 10:13:35 +0100
> Anton Lindqvist <an...@openbsd.org> wrote:
> 
> > On Fri, Nov 10, 2017 at 09:36:25AM +0100, Martin Pieuchot wrote:
> > > On 09/11/17(Thu) 09:02, Helg Bredow wrote:
> > > > The current libfuse signal handling assumes that the file system will 
> > > > always be unmounted by the child. One obvious case where this is not 
> > > > true is if the file system is busy. To replicate:
> > > > 
> > > > 1. mount a fuse file system
> > > > 2. cd anywhere on the file system
> > > > 3. pkill -INT 
> > > > 
> > > > The result is a zombie child process and no more response to signals 
> > > > even if the file system is no longer busy.
> > > > 
> > > > This patch ensures that the child always exits and that an error is 
> > > > printed to stdout if the file system cannot be unmounted. Tested with 
> > > > fuse-exfat and ntfs_3g. Suggestions for improvement are welcome.
> > > 
> > > Nice to see that you're fixing a bug.  However I'd suggest you to go
> > > much further in your improvement.
> > > 
> > > Signal handlers are hard and instead of doing work inside the signal
> > > handler you should toggle a global variable/flag and do this work
> > > inside the main loop (fuse_loop()?).
> > > 
> > > For example your code below calls fprintf(3) in the signal handler.  This
> > > is incorrect, this functions is not signal handler safe.  What about 
> > > fuse_unmount()?  Are you sure it can be called from a signal handler?
> > 
> > Some more info on making signal handlers asynchronous-safe:
> > 
> > https://www.securecoding.cert.org/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers
> 
> Thanks for the feedback and info guys. I wasn't too confident with this patch 
> and you've given me some good pointers to improve it.
> 
> -- 
> Helg <xx...@msn.com>
> 

I've completely rewritten the patch. Is this better?


Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.34
diff -u -p -u -p -r1.34 fuse.c
--- fuse.c  4 Nov 2017 13:17:18 -   1.34
+++ fuse.c  12 Nov 2017 04:28:21 -
@@ -31,7 +31,7 @@
 #include "fuse_private.h"
 #include "debug.h"
 
-static struct fuse_session *sigse;
+static volatile sig_atomic_t signum = 0;
 static struct fuse_context *ictx = NULL;
 static int max_read = FUSEBUFMAXSIZE;
 
@@ -61,6 +61,48 @@ static struct fuse_opt fuse_core_opts[] 
FUSE_OPT_END
 };
 
+static void
+ifuse_get_signal(int num)
+{
+   signum = num;
+}
+
+static void
+ifuse_child_exit(const struct fuse *f)
+{
+   int status;
+
+   signal(SIGCHLD, SIG_DFL);
+   if (waitpid(WAIT_ANY, , WNOHANG) == -1)
+   fprintf(stderr, "fuse: %s\n", strerror(errno));
+
+   if (WIFEXITED(status) && (WEXITSTATUS(status) != 0))
+   fprintf(stderr, "fuse: %s: %s\n",
+   f->fc->dir, strerror(WEXITSTATUS(status)));
+
+   return;
+}
+
+static void
+ifuse_try_unmount(const struct fuse *f)
+{
+   pid_t child;
+
+   signal(SIGCHLD, ifuse_get_signal);
+   child = fork();
+
+   if (child < 0) {
+   DPERROR(__func__);
+   return;
+   }
+
+   if (child == 0) {
+   errno = 0;
+   fuse_unmount(f->fc->dir, f->fc);
+   _exit(errno);
+   }
+}
+
 int
 fuse_loop(struct fuse *fuse)
 {
@@ -83,9 +125,24 @@ fuse_loop(struct fuse *fuse)
 
while (!fuse->fc->dead) {
ret = kevent(fuse->fc->kq, >fc->event, 1, , 1, NULL);
-   if (ret == -1)
-   DPERROR(__func__);
-   else if (ret > 0) {
+   if (ret == -1) {
+   if (errno == EINTR) {
+   switch (signum) {
+   case SIGCHLD:
+   ifuse_child_exit(fuse);
+   break;
+   case SIGHUP:
+   case SIGINT:
+   case SIGTERM:
+   ifuse_try_unmount(fuse);
+   break;
+   default:
+   fprintf(stderr, "%s: %s\n", __func__,
+   strsignal(signum));
+   }
+   } else
+ 

Re: libfuse: signal handler doesn't cater for "Device busy" and other errors

[Helg Bredow]

On Fri, 10 Nov 2017 10:13:35 +0100
Anton Lindqvist <an...@openbsd.org> wrote:

> On Fri, Nov 10, 2017 at 09:36:25AM +0100, Martin Pieuchot wrote:
> > On 09/11/17(Thu) 09:02, Helg Bredow wrote:
> > > The current libfuse signal handling assumes that the file system will 
> > > always be unmounted by the child. One obvious case where this is not true 
> > > is if the file system is busy. To replicate:
> > > 
> > > 1. mount a fuse file system
> > > 2. cd anywhere on the file system
> > > 3. pkill -INT 
> > > 
> > > The result is a zombie child process and no more response to signals even 
> > > if the file system is no longer busy.
> > > 
> > > This patch ensures that the child always exits and that an error is 
> > > printed to stdout if the file system cannot be unmounted. Tested with 
> > > fuse-exfat and ntfs_3g. Suggestions for improvement are welcome.
> > 
> > Nice to see that you're fixing a bug.  However I'd suggest you to go
> > much further in your improvement.
> > 
> > Signal handlers are hard and instead of doing work inside the signal
> > handler you should toggle a global variable/flag and do this work
> > inside the main loop (fuse_loop()?).
> > 
> > For example your code below calls fprintf(3) in the signal handler.  This
> > is incorrect, this functions is not signal handler safe.  What about 
> > fuse_unmount()?  Are you sure it can be called from a signal handler?
> 
> Some more info on making signal handlers asynchronous-safe:
> 
> https://www.securecoding.cert.org/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers

Thanks for the feedback and info guys. I wasn't too confident with this patch 
and you've given me some good pointers to improve it.

-- 
Helg <xx...@msn.com>

libfuse: signal handler doesn't cater for "Device busy" and other errors

[Helg Bredow]

The current libfuse signal handling assumes that the file system will always be 
unmounted by the child. One obvious case where this is not true is if the file 
system is busy. To replicate:

1. mount a fuse file system
2. cd anywhere on the file system
3. pkill -INT 

The result is a zombie child process and no more response to signals even if 
the file system is no longer busy.

This patch ensures that the child always exits and that an error is printed to 
stdout if the file system cannot be unmounted. Tested with fuse-exfat and 
ntfs_3g. Suggestions for improvement are welcome.

Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.34
diff -u -p -r1.34 fuse.c
--- fuse.c  4 Nov 2017 13:17:18 -   1.34
+++ fuse.c  9 Nov 2017 08:41:11 -
@@ -303,26 +303,40 @@ ifuse_get_signal(unused int num)
 {
struct fuse *f;
pid_t child;
-   int status;
+   int status, err;
+
+   if (num == SIGCHLD) {
+   /* child fuse_unmount() completed, check error */
+   while (waitpid(WAIT_ANY, , WNOHANG) == -1) {
+   if (errno != EINTR)
+   fprintf(stderr, "fuse: %s\n", strerror(errno));
+   }
+
+   if (WIFEXITED(status)) {
+   err = WEXITSTATUS(status);
+   if (err)
+   fprintf(stderr, "fuse: %s\n", strerror(err));
+   }
+
+   signal(SIGCHLD, SIG_DFL);
+   return;
+   }
 
if (sigse != NULL) {
+   signal(SIGCHLD, ifuse_get_signal);
child = fork();
 
if (child < 0)
-   return;
+   return ;
 
-   f = sigse->args;
if (child == 0) {
+   /* try to unmount, file system may be busy */
+   f = sigse->args;
+   errno = 0;
fuse_unmount(f->fc->dir, f->fc);
-   sigse = NULL;
-   exit(0);
+   _exit(errno);
}
 
-   fuse_loop(f);
-   while (waitpid(child, , 0) == -1) {
-   if (errno != EINTR)
-   break;
-   }
}
 }
 

Re: fuse: vfs create does not map 1:1 to fuse create

[Helg Bredow]

On Wed, 8 Nov 2017 16:50:07 +0100
Martin Pieuchot <m...@openbsd.org> wrote:

> On 08/11/17(Wed) 14:12, Helg Bredow wrote:
> > There is a bug when creating a file in fuse-exfat and then deleting it
> > again without first unmounting the file system. The reason for this is
> > that fuse-exfat maintains strict reference counts and fuse currently
> > calls the file system create and open functions when it should only
> > call create. 
> > [...]
> 
> Does it call the userland file system functions because it receives 2
> FBT messages?

Yes, fuse receives both create and open from the kernel and blindly
passes them via FBT messages to userland.

> 
> Can you see which FBT messages are generated by the kernel?  Does
> ktrace(1) has support for that?

The debug output from libfuse let's you trace the FBT messages and I
also have judicious printf() statements to help me diagnose.

> 
> > The VOP_CREATE(9) function does not behave like this so we either need
> > to simulate it within fuse or fall back to mknod() and open(). 
> 
> VOP_CREATE(9) is just a wrapper around the underlying FS.  In your case
> fusefs_create().  This function enqueues a single FBT_CREATE operation,
> so it doesn't match what you said before.
> 
> So I still don't understand the problem.  Where is the bug?
> 

The problem is that the kernel's idea of create is different to the
fuse FBT_CREATE passed to userland. The kernel doesn't expect create to
also open the file. fuse create was designed to map 1:1 to
atomic_open, which OpenBSD does not have and I'm not keen to implement
it.

> If the problem is that the kernel enqueue 2 FBT operations instead of
> one, then change the kernel.
> 
> If the problem is that fusefs_create() does not have all the information
> for generating a FBT_CREATE message and that it hardcodes (O_CREAT|O_RDWR)
> then maybe you should delay that operation to fusefs_open().
> 
> These are stupid guesses because I don't understand what the problem is.

Your last suggestion is one that I had not considered. I immediately
went to implement it but was disappointed to find that it won't work
without crazy lock management and maintaining state between kernel
calls. I think it's too complex and prefer the simple solution I've
proposed because it behaves the same as ffs, nfs etc.

-- 
Helg <xx...@msn.com>

fuse: vfs create does not map 1:1 to fuse create

[Helg Bredow]

There is a bug when creating a file in fuse-exfat and then deleting it
again without first unmounting the file system. The reason for this is
that fuse-exfat maintains strict reference counts and fuse currently
calls the file system create and open functions when it should only
call create. The additional call to open results in a node having one
extra reference on release.

The Linux version of fuse.h states the following for create.

* Create and open a file
*
* If the file does not exist, first create it with the specified
* mode, and then open it.
*
* If this method is not implemented or under Linux kernel
* versions earlier than 2.6.15, the mknod() and open() methods
* will be called instead.
*
* Introduced in version 2.5

The VOP_CREATE(9) function does not behave like this so we either need
to simulate it within fuse or fall back to mknod() and open(). Note
that fuse mknod DOES allow the creation of regular files so doesn't map
completely 1:1 to OpenBSDs mknod(2). Linux has the atomic_open system
call, which maps 1:1 to fuse create.

The following patch is the first pass at changing the behaviour of
fusefs_create so that it maps 1:1 to fuse mknod. If this approach is
accepted, we should consider lowering FUSE_VERSION in fuse.h to 24 so
that file systems can check the supported fuse version at compile time
to determine whether create is available. The only port that currently
does not implement mknod is fuse-zip. It should not be difficult to
patch this.

An alternative and more complicated patch is to modify fusefs_create to
store the file handle returned by the file system in the same way that
fusefs_open does and then not open the file again. The downside of this
is that fusefs_create does not receive the open flags from the vn_open
vfs system call so cannot pass these onto the fuse file system. Let me
know if you would like to see this alternative patch.


Index: sys/miscfs/fuse/fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.33
diff -u -p -r1.33 fuse_vnops.c
--- sys/miscfs/fuse/fuse_vnops.c7 Sep 2016 17:53:35
-   1.33 +++ sys/miscfs/fuse/fuse_vnops.c   8 Nov 2017
13:38:04 - @@ -891,13 +891,13 @@ fusefs_create(void *v)
goto out;
}
 
-   if (fmp->undef_op & UNDEF_CREATE) {
+   if (fmp->undef_op & UNDEF_MKNOD) {
error = ENOSYS;
goto out;
}
 
fbuf = fb_setup(cnp->cn_namelen + 1, ip->ufs_ino.i_number,
-   FBT_CREATE, p);
+   FBT_MKNOD, p);
 
fbuf->fb_io_mode = mode;
fbuf->fb_io_flags = O_CREAT | O_RDWR;
@@ -908,7 +908,7 @@ fusefs_create(void *v)
error = fb_queue(fmp->dev, fbuf);
if (error) {
if (error == ENOSYS)
-   fmp->undef_op |= UNDEF_CREATE;
+   fmp->undef_op |= UNDEF_MKNOD;
 
fb_delete(fbuf);
goto out;
Index: lib/libfuse/fuse_ops.c
===
RCS file: /cvs/src/lib/libfuse/fuse_ops.c,v
retrieving revision 1.26
diff -u -p -r1.26 fuse_ops.c
--- lib/libfuse/fuse_ops.c  7 Sep 2016 17:53:35 -   1.26
+++ lib/libfuse/fuse_ops.c  8 Nov 2017 13:38:05 -
@@ -598,8 +598,6 @@ ifuse_ops_create(struct fuse *f, struct 
 
if (f->op.create)
fbuf->fb_err = f->op.create(realname, mode,  );
-   else if (f->op.mknod)
-   fbuf->fb_err = f->op.mknod(realname, S_IFREG | mode,
0); else
fbuf->fb_err = -ENOSYS;
 

libfuse: fuse.c uses Incorrect version macro

[Helg Bredow]

The fuse_version() library function incorrectly returns FUSE_USE_VERSION. This 
macro is intended to be used by file systems to indicate the version of the 
libfuse API to compile against. This change returns the highest version of the 
API that libfuse supports. Both macros have the same value so this change is 
pedantic only.

Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.32
diff -u -p -r1.32 fuse.c
--- fuse.c  2 Nov 2017 13:55:37 -   1.32
+++ fuse.c  4 Nov 2017 00:30:24 -
@@ -362,7 +362,7 @@ dump_help(void)
 static void
 dump_version(void)
 {
-   fprintf(stderr, "FUSE library version %i\n", FUSE_USE_VERSION);
+   fprintf(stderr, "FUSE library version %i\n", FUSE_VERSION);
 }
 
 static int

libfuse: fuse_loop_mt() should return -1

[Helg Bredow]

The library function fuse_loop_mt() is currently unimplemented. It should 
return -1 to indicate failure rather than claim success when it clearly didn't 
do anything useful. fuse_loop_mt() should never be called by file systems since 
fuse_parse_cmdline() and fuse_setup() alway return 0 as the value of the mt 
back to callers.

Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.32
diff -u -p -r1.32 fuse.c
--- fuse.c  2 Nov 2017 13:55:37 -   1.32
+++ fuse.c  3 Nov 2017 17:33:58 -
@@ -236,7 +236,7 @@ fuse_get_session(struct fuse *f)
 int
 fuse_loop_mt(unused struct fuse *fuse)
 {
-   return (0);
+   return (-1);
 }
 
 struct fuse *

Re: libfuse: improved command line parsing

[Helg Bredow]

On Sat, 28 Oct 2017 10:07:39 +0200
Martin Pieuchot <m...@openbsd.org> wrote:

> On 17/10/17(Tue) 14:26, Helg Bredow wrote:
> > [...] 
> > I've split the patch. This one improves argument and option parsing so that 
> > almost all sshfs arguments and options will now parse. It won't recognise 
> > -ocache=no since fuse_opt_match() is incorrect (fuse will treat it the same 
> > as -ocache=yes). I'll submit a patch for that some other time.
> 
> Nice.
> 
> > Running check_sym tells me:
> > 
> > No dynamic export changes
> > External reference changes:
> > added:
> > atoi
> > strchr
> > strsep
> > strstr
> > strtoul
> > 
> > I take that to mean that there is no need to bump the major or minor 
> > version for this patch. Is that correct?
> > 
> 
> That's correct.
> 
> Regarding fuse options, I'd suggest writing more tests and fuzzing this
> code.  Complex argument parsing is hard to get right.
> 

I've tried fuzzing this logic using AFL, indirectly by calling 
fuse_parse_cmdline() but I'm not sure it's that useful. It'll just end up 
supplying nonsensical arguments that never match and won't execute much of the 
other code paths. I've tested this pretty exhaustively and am confident that 
it's as stable as I can make it. I'll post the regression tests that I've been 
using in a separate email. All the changes that I'm making to fuse are a result 
of testing and not auditing the code.

I agree that this is hard to get right and it took me a few attempts to 
simplify. Your comments did prompt me to try a few more tests and I've found 
and fixed another edge case. It now prints a useful message if you don't supply 
a value to an argument that expects a value. e.g. "sshfs -p".
 
> Comments inline.

Responses and updated patch below.

> 
> > Index: fuse_opt.c
[...]
> > -   keyval = 0;
> > +#define DISCARD 0
> > +#define KEEP 1
> > +#define NEED_ANOTHER_ARG 2
> 
> I'd prefix these defines with _FOPT or w/o underscore if they are part
> of the API and move them on the top of the file since you use them in
> multiple functions.
>

Done.
 
> >  
> > -   /* check if it is a key=value entry */
> > -   idx = strcspn(val, "=");
> > -   if (idx != strlen(val)) {
> > -   idx++;
> > -   keyval = 1;
> > -   }
> > +   if (o == NULL)
> > +   return KEEP;
> 
> I know it's note your code, but one-letter variable should be avoided.
> It's hard to understand what they represent.  But let's not clutter this
> diff :)
> 

Agreed. I've been careful not to tidy the code at the same time.

> > +
> > +   keyval = 0;
> >  
> > for(; o->templ; o++) {
> > -   if ((keyval && strncmp(val, o->templ, idx) == 0) ||
> > -   (!keyval && strcmp(val, o->templ) == 0)) {
> > +   /* check key=value or -p n */
> > +   idx = strcspn(o->templ, "= ");
> > +
> > +   if (strncmp(val, o->templ, idx) == 0) {
> > +   if (o->templ[idx] == '=') {
> > +   keyval = 1;
> > +   val = [idx + 1];
> 
> How can you be sure that val[idx + 1] is valid?  For example:
> 
>   o->temp = { 'k', 'e', 'y', '=', 'v', 'a', 'l', '\0' }
>   val = { 'k', '\0' }

This example is not possible. The previous strncmp() test guarantees that both 
0->templ and val are identical up to either a '=' or ' '. val[idx + 1] will at 
worst equal '\0'.

   o->temp = { 'k', 'e', 'y', '=', 'v', 'a', 'l', '\0' }
   val = { 'k', 'e', 'y', '=', '\0' }

> 
> This looks like an overflow to me.  Well not yet, but when you
> dereference `val' below.
> 
> > +   } else if (o->templ[idx] == ' ') {
> > +   keyval = 1;
> > +   if (idx == strlen(val)) {
> > +   /* ask for next arg to be included */
> > +   return NEED_ANOTHER_ARG;
> > +   } else if (strchr(o->templ, '%') != NULL) {
> > +   val = [idx];
> 
> Same here, how can you be sure that val[idx] is valid?

Worst case it will be '\0'. Even by supplying a matching 'opt=' or '-p' I can't 
get it to crash.

> 
> > +   }
> > +   }
> > +
> > if (o->val == FUSE_OPT_KEY_DISCARD)
> > -   return (1);
> > +   return (DISCARD);
> > +
> > +   if (o->val =

Re: libfuse: fuse.c null checks and others

[Helg Bredow]

On Sat, 28 Oct 2017 09:24:55 +0200
Martin Pieuchot <m...@openbsd.org> wrote:

> On 25/10/17(Wed) 13:27, Helg Bredow wrote:
> > I've included different minor patches below as one patch. I haven't split 
> > into separate patches since the changes are not complex and easy to audit. 
> > 
> > Here's what it does:
> > 
> > Almost all functions in fuse.c do not check if the arguments are null. This 
> > patch adds null checks where appropriate.
> > 
> > Some arguments are incorrectly flagged as unused. Delete "unused" if the 
> > argument is used in the function.
> > 
> > The wrong version macro is used in dump_version() and is inconsistent with 
> > that used in fuse_version(). FUSE_USE_VERSION is intended to be defined by 
> > file system to specify which backward compatible version of libfuse they 
> > require.
> > 
> > fuse_loop_mt() is not implemented so return -1 rather than success. If a 
> > file system tries to call it then it should be obvious that it's not going 
> > to work.
> > 
> > fuse_main() declared redundant variables due to the lack of NULL checks 
> > before assignment. We now check for NULL so can safely pass NULL instead.
> > 
> > Tested with a regression test that passes all NULL arguments to exported 
> > functions.
> > 
> > Something to consider is that fuse_is_lib_option() is deprecated. Should we 
> > remove it from libfuse to stay strictly at version 26?
> 
> Testing for NULL is good.  However returning -1 in fuse_loop_mt() and
> changing the version number might break ports.  So if these changes go
> in, you should watch for regression on ports@ at least.

I've reverted fuse_loop_mt(). However, I don't feel that this is correct. If a 
file system expects it to work then it should fail to make it clear that the 
functionality is not implemented. sshfs calls it but because 
fuse_parse_cmdline() always returns 0 for multithreaded it never gets called 
(you normally need to specify -s on the command line for multithreaded fuse 
file systems to run in a single thread. I've tested to see what sshfs does when 
fuse_loop_mt() returns -1 and it simply exits with no message, just as it does 
when it returns 0.

I also reverted the version macro change. The value for both macros is the same 
(26) and they will likely stay in step if the version is incremented. It's not 
using the correct macro though.

> 
> We can remove fuse_is_lib_option() if nothing in ports use it.  A good
> start to search for it is codesearch.debian.net.  You can also ask
> porters to do a grep on unpacked port tree.
> 

I didn't know about this site, it will be useful. I was going to search the 
ports source tree but this saves me the trouble and also expands the search. 
sshfs calls this fuse_is_lib_option() so it will have to stay. However, it also 
needs work as it doesn't behave the same as the Linux version.

> Could you provide a diff including only the NULL check and the removal
> of "unused" markers?
> 

Updated patch is below. I had missed a NULL check in fuse_set_signal_handlers() 
so have added it. The signal handling only works if the file system is not 
busy, otherwise it becomes very difficult to kill. fuse_loop() also doesn't set 
the signal handlers but it should. More things to fix later.

Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.31
diff -u -p -r1.31 fuse.c
--- fuse.c  25 Oct 2017 09:29:46 -  1.31
+++ fuse.c  28 Oct 2017 13:39:26 -
@@ -71,6 +71,9 @@ fuse_loop(struct fuse *fuse)
ssize_t n;
int ret;
 
+   if (fuse == NULL)
+   return (-1);
+
fuse->fc->kq = kqueue();
if (fuse->fc->kq == -1)
return (-1);
@@ -156,6 +159,9 @@ fuse_mount(const char *dir, unused struc
struct fuse_chan *fc;
const char *errcause;
 
+   if (dir == NULL)
+   return (NULL);
+
fc = calloc(1, sizeof(*fc));
if (fc == NULL)
return (NULL);
@@ -197,9 +203,9 @@ bad:
 }
 
 void
-fuse_unmount(const char *dir, unused struct fuse_chan *ch)
+fuse_unmount(const char *dir, struct fuse_chan *ch)
 {
-   if (ch->dead)
+   if (ch == NULL || ch->dead)
return;
 
if (unmount(dir, MNT_UPDATE) == -1)
@@ -207,7 +213,7 @@ fuse_unmount(const char *dir, unused str
 }
 
 int
-fuse_is_lib_option(unused const char *opt)
+fuse_is_lib_option(const char *opt)
 {
return (fuse_opt_match(fuse_core_opts, opt));
 }
@@ -215,6 +221,9 @@ fuse_is_lib_option(unused const char *op
 int
 fuse_chan_fd(struct fuse_chan *ch)
 {
+   if (ch == NULL)
+   return (-1);
+
return (ch->fd);
 }
 
@@ -233,11 +242,14 @@ 

libfuse: regress test for command line parsing

[Helg Bredow]

These tests confirm that currently supported options are parsed correctly by 
libfuse. They assume that the patches I've submitted previously have been 
applied.

Review and suggestions for improvement are welcome.

Patch to Makefile is below and new tests are attached.


Index: Makefile
===
RCS file: /cvs/src/regress/lib/libfuse/Makefile,v
retrieving revision 1.1
diff -u -p -p -u -r1.1 Makefile
--- Makefile9 Aug 2013 16:20:10 -   1.1
+++ Makefile29 Oct 2017 08:44:36 -
@@ -5,6 +5,9 @@ REGRESS_TARGETS+= run-fuse-opt-add-opt-e
 REGRESS_TARGETS+= run-fuse-opt-add-arg
 REGRESS_TARGETS+= run-fuse-opt-insert-arg
 REGRESS_TARGETS+= run-fuse-opt-match
+REGRESS_TARGETS+= run-fuse-opt-parse
+REGRESS_TARGETS+= run-fuse-parse-cmdline
 
 LDFLAGS+= -lfuse
 CLEANFILES= fuse-opt-add-opt
@@ -12,6 +15,9 @@ CLEANFILES+=fuse-opt-add-opt-escaped
 CLEANFILES+=fuse-opt-add-arg
 CLEANFILES+=fuse-opt-insert-arg
 CLEANFILES+=fuse-opt-match
+CLEANFILES+=fuse-opt-parse
+CLEANFILES+=fuse-parse-cmdline
 
 .PHONY: ${REGRESS_TARGETS}
 
@@ -25,5 +31,11 @@ run-fuse-opt-insert-arg: fuse-opt-insert
./fuse-opt-insert-arg
 run-fuse-opt-match: fuse-opt-match
./fuse-opt-match
+run-fuse-opt-parse: fuse-opt-parse
+   ./fuse-opt-parse
+run-fuse-parse-cmdline: fuse-parse-cmdline
+   ./fuse-parse-cmdline
 
 .include 


fuse-opt-parse.c
Description: fuse-opt-parse.c


fuse-parse-cmdline.c
Description: fuse-parse-cmdline.c

libfuse: fuse.c null checks and others

[Helg Bredow]

I've included different minor patches below as one patch. I haven't split into 
separate patches since the changes are not complex and easy to audit. 

Here's what it does:

Almost all functions in fuse.c do not check if the arguments are null. This 
patch adds null checks where appropriate.

Some arguments are incorrectly flagged as unused. Delete "unused" if the 
argument is used in the function.

The wrong version macro is used in dump_version() and is inconsistent with that 
used in fuse_version(). FUSE_USE_VERSION is intended to be defined by file 
system to specify which backward compatible version of libfuse they require.

fuse_loop_mt() is not implemented so return -1 rather than success. If a file 
system tries to call it then it should be obvious that it's not going to work.

fuse_main() declared redundant variables due to the lack of NULL checks before 
assignment. We now check for NULL so can safely pass NULL instead.

Tested with a regression test that passes all NULL arguments to exported 
functions.

Something to consider is that fuse_is_lib_option() is deprecated. Should we 
remove it from libfuse to stay strictly at version 26?


Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.31
diff -u -p -r1.31 fuse.c
--- fuse.c  25 Oct 2017 09:29:46 -  1.31
+++ fuse.c  25 Oct 2017 12:54:48 -
@@ -71,6 +71,9 @@ fuse_loop(struct fuse *fuse)
ssize_t n;
int ret;
 
+   if (fuse == NULL)
+   return (-1);
+
fuse->fc->kq = kqueue();
if (fuse->fc->kq == -1)
return (-1);
@@ -156,6 +159,9 @@ fuse_mount(const char *dir, unused struc
struct fuse_chan *fc;
const char *errcause;
 
+   if (dir == NULL)
+   return (NULL);
+
fc = calloc(1, sizeof(*fc));
if (fc == NULL)
return (NULL);
@@ -197,9 +203,9 @@ bad:
 }
 
 void
-fuse_unmount(const char *dir, unused struct fuse_chan *ch)
+fuse_unmount(const char *dir, struct fuse_chan *ch)
 {
-   if (ch->dead)
+   if (ch == NULL || ch->dead)
return;
 
if (unmount(dir, MNT_UPDATE) == -1)
@@ -207,7 +213,7 @@ fuse_unmount(const char *dir, unused str
 }
 
 int
-fuse_is_lib_option(unused const char *opt)
+fuse_is_lib_option(const char *opt)
 {
return (fuse_opt_match(fuse_core_opts, opt));
 }
@@ -215,6 +221,9 @@ fuse_is_lib_option(unused const char *op
 int
 fuse_chan_fd(struct fuse_chan *ch)
 {
+   if (ch == NULL)
+   return (-1);
+
return (ch->fd);
 }
 
@@ -227,17 +236,20 @@ fuse_get_session(struct fuse *f)
 int
 fuse_loop_mt(unused struct fuse *fuse)
 {
-   return (0);
+   return (-1);
 }
 
 struct fuse *
 fuse_new(struct fuse_chan *fc, unused struct fuse_args *args,
 const struct fuse_operations *ops, unused size_t size,
-unused void *userdata)
+void *userdata)
 {
struct fuse *fuse;
struct fuse_vnode *root;
 
+   if (fc == NULL || ops == NULL)
+   return (NULL);
+
if ((fuse = calloc(1, sizeof(*fuse))) == NULL)
return (NULL);
 
@@ -275,8 +287,11 @@ fuse_daemonize(unused int foreground)
 }
 
 void
-fuse_destroy(unused struct fuse *f)
+fuse_destroy(struct fuse *f)
 {
+   if (f == NULL)
+   return;
+
close(f->fc->fd);
free(f->fc->dir);
free(f->fc);
@@ -322,7 +337,7 @@ fuse_remove_signal_handlers(unused struc
 }
 
 int
-fuse_set_signal_handlers(unused struct fuse_session *se)
+fuse_set_signal_handlers(struct fuse_session *se)
 {
sigse = se;
signal(SIGHUP, ifuse_get_signal);
@@ -344,7 +359,7 @@ dump_help(void)
 static void
 dump_version(void)
 {
-   fprintf(stderr, "FUSE library version %i\n", FUSE_USE_VERSION);
+   fprintf(stderr, "FUSE library version %i\n", FUSE_VERSION);
 }
 
 static int
@@ -453,6 +468,9 @@ fuse_version(void)
 void
 fuse_teardown(struct fuse *fuse, char *mp)
 {
+   if (fuse == NULL || mp == NULL)
+   return;
+
fuse_unmount(mp, fuse->fc);
fuse_destroy(fuse);
 }
@@ -500,10 +518,8 @@ int
 fuse_main(int argc, char **argv, const struct fuse_operations *ops, void *data)
 {
struct fuse *fuse;
-   char *mp = NULL;
-   int mt;
 
-   fuse = fuse_setup(argc, argv, ops, sizeof(*ops), , , data);
+   fuse = fuse_setup(argc, argv, ops, sizeof(*ops), NULL, NULL, data);
if (!fuse)
return (-1);
 

fuselib: patch to check for NULL in fuse_parse_cmdline()

[Helg Bredow]

Arguments to fuse_parse_cmdline() are not checked for NULL before assignment. 
This patch performs the check.

Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.30
diff -u -p -u -p -r1.30 fuse.c
--- fuse.c  24 Oct 2017 09:01:05 -  1.30
+++ fuse.c  24 Oct 2017 13:12:51 -
@@ -426,10 +426,14 @@ fuse_parse_cmdline(struct fuse_args *arg
return (-1);
}
 
-   *mp = strdup(opt.mp);
-   if (*mp == NULL)
-   return (-1);
-   *mt = 0;
+   if (mp != NULL) {
+   *mp = strdup(opt.mp);
+   if (*mp == NULL)
+   return (-1);
+   }
+
+   if (mt != NULL)
+   *mt = 0;
 
return (0);
 }

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

[Helg Bredow]

On Wed, 18 Oct 2017 15:03:21 +0200
Martin Pieuchot <m...@openbsd.org> wrote:

> On 18/10/17(Wed) 12:51, Helg Bredow wrote:
> > 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.
> 
> But what if mp is NULL?

Good point; I hadn't considered that. I've added the check.

> 
> > `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).
> 
> Nice.


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 -  1.29
+++ fuse.c  18 Oct 2017 14:30:08 -
@@ -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(, mp, mt, ))
+   dir = NULL;
+   if (fuse_parse_cmdline(, , mt, ))
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,12 @@ fuse_setup(int argc, char **argv, const 
goto err;
}
 
+   if (mp != NULL)
+   *mp = dir;
+
return (fuse);
 err:
-   free(*mp);
+   free(dir);
return (NULL);
 }
 

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 -  1.29
+++ fuse.c  18 Oct 2017 12:39:12 -
@@ -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(, mp, mt, ))
+   dir = NULL;
+   if (fuse_parse_cmdline(, , mt, ))
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);
 }
 

libfuse: patch to prevent fuse-zip from dumping core

[Helg Bredow]

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.

Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.29
diff -u -p -u -p -r1.29 fuse.c
--- fuse.c  21 Aug 2017 21:41:13 -  1.29
+++ fuse.c  17 Oct 2017 15:21:05 -
@@ -468,6 +468,7 @@ fuse_setup(int argc, char **argv, const 
struct fuse *fuse;
int fg;
 
+   *mp = NULL;
if (fuse_parse_cmdline(, mp, mt, ))
goto err;
 

Re: libfuse: improved command line parsing

[Helg Bredow]

On Mon, 16 Oct 2017 15:33:45 +0200
Martin Pieuchot <m...@openbsd.org> wrote:

> On 14/10/17(Sat) 01:23, Helg Bredow wrote:
> > The attached patch vastly improves fuse argument and option parsing. For 
> > example, all sshfs options will now be parsed successfully. It's a 
> > substantial patch and can't be easily broken down further.
> > 
> > In addition, this also adds support for the -d and -odebug options.
> > 
> > I've also included a regression test that confirms it's working as intended.
> > 
> > The only contentious part of the patch is that fuse no longer fails if it 
> > doesn't recognise an argument or option, it only prints a message to 
> > stdout. This is because fuse file systems can pass additional options to 
> > fuse and most are not supported and/or recognised yet and this would 
> > prevent those file systems from running.
> > 
> > I'm happy to take feedback and recommendations.
> 
> Your diffs are good but it would be easier if you could submit one diff
> per issue.
> 
> For example, split the debug part, the foreground part, the options and
> the regress.  This will allow us to integrate some non contentious part
> of your diff then discuss the rest.
> 
> You also want to have a look at /usr/lib/check_sym since you're adding
> new symbols to the library.

I've split the patch. This one improves argument and option parsing so that 
almost all sshfs arguments and options will now parse. It won't recognise 
-ocache=no since fuse_opt_match() is incorrect (fuse will treat it the same as 
-ocache=yes). I'll submit a patch for that some other time.

Running check_sym tells me:

No dynamic export changes
External reference changes:
added:
atoi
strchr
strsep
strstr
strtoul

I take that to mean that there is no need to bump the major or minor version 
for this patch. Is that correct?

Index: fuse_opt.c
===
RCS file: /cvs/src/lib/libfuse/fuse_opt.c,v
retrieving revision 1.18
diff -u -p -u -p -r1.18 fuse_opt.c
--- fuse_opt.c  4 Jan 2017 12:01:22 -   1.18
+++ fuse_opt.c  17 Oct 2017 13:57:35 -
@@ -222,57 +222,70 @@ static int
 parse_opt(const struct fuse_opt *o, const char *val, void *data,
 fuse_opt_proc_t f, struct fuse_args *arg)
 {
-   int found, ret, keyval;
+   int keyval;
size_t idx;
 
-   ret = 0;
-   found = 0;
-   keyval = 0;
+#define DISCARD 0
+#define KEEP 1
+#define NEED_ANOTHER_ARG 2
 
-   /* check if it is a key=value entry */
-   idx = strcspn(val, "=");
-   if (idx != strlen(val)) {
-   idx++;
-   keyval = 1;
-   }
+   if (o == NULL)
+   return KEEP;
+
+   keyval = 0;
 
for(; o->templ; o++) {
-   if ((keyval && strncmp(val, o->templ, idx) == 0) ||
-   (!keyval && strcmp(val, o->templ) == 0)) {
+   /* check key=value or -p n */
+   idx = strcspn(o->templ, "= ");
+
+   if (strncmp(val, o->templ, idx) == 0) {
+   if (o->templ[idx] == '=') {
+   keyval = 1;
+   val = [idx + 1];
+   } else if (o->templ[idx] == ' ') {
+   keyval = 1;
+   if (idx == strlen(val)) {
+   /* ask for next arg to be included */
+   return NEED_ANOTHER_ARG;
+   } else if (strchr(o->templ, '%') != NULL) {
+   val = [idx];
+   }
+   }
+
if (o->val == FUSE_OPT_KEY_DISCARD)
-   return (1);
+   return (DISCARD);
+
+   if (o->val == FUSE_OPT_KEY_KEEP)
+   return (KEEP);
 
if (FUSE_OPT_IS_OPT_KEY(o)) {
-   if (keyval)
-   ret = f(data, [idx], o->val, arg);
-   else
-   ret = f(data, val, o->val, arg);
-   }
+   if (f == NULL)
+   return KEEP;
 
-   if (o->off != ULONG_MAX && data && o->val >= 0) {
-   ret = f(data, val, o->val, arg);
-   int *addr = (int *)(data + o->off);
-   *addr = o->val;
-   ret = 0;
+   return f(data, val, o->val, arg);
+ 

libfuse: improved command line parsing

[Helg Bredow]

The attached patch vastly improves fuse argument and option parsing. For 
example, all sshfs options will now be parsed successfully. It's a substantial 
patch and can't be easily broken down further.

In addition, this also adds support for the -d and -odebug options.

I've also included a regression test that confirms it's working as intended.

The only contentious part of the patch is that fuse no longer fails if it 
doesn't recognise an argument or option, it only prints a message to stdout. 
This is because fuse file systems can pass additional options to fuse and most 
are not supported and/or recognised yet and this would prevent those file 
systems from running.

I'm happy to take feedback and recommendations.

-- 
Helg <xx...@msn.com>


libfuse-20171014.diff
Description: libfuse-20171014.diff


fuse-opt-parse-regress.diff
Description: fuse-opt-parse-regress.diff

Re: FUSE Patches

[Helg]

On Sat, 21 Feb 2015 12:56:00 +0100
Stefan Sperling s...@stsp.name wrote:

 On Sun, Feb 22, 2015 at 12:55:35AM +0800, Helg wrote:
  Hi All,
  
  I’ve spent a bit more time hacking on fuse and while it still has some
  way to go, the attached patch moves it forward in the right direction.
  I’d appreciate it if people can test and provide feedback.
 
 Since the tree is locked for release only critical bug fixes and
 fixes for things which are new in 5.7 are being worked on right now.
 So if you don't get much feedback now please come back later.
 
 You should consider splitting your patches up into smaller chunks
 which can be tested and reviewed separately.

I thought I might be a bit late with this but thanks for letting me
know. I'll keep working on it and submit the patches individually once
5.7 is released.

  I’ve tested on amd64 under VirtualBox and GNU Configure proved to be an
  excellent stress test and with this patch, running this on an ntfs-3g
  mounted file system no longer causes fuse to hang.
 
 Checking out and updating large Subversion and Git trees are also good
 test cases that I've used in the past.

Good to know; I'll include this as part of my tests.

-- 
Helg xx...@msn.com

FUSE Patches

[Helg]

Hi All,

I’ve spent a bit more time hacking on fuse and while it still has some
way to go, the attached patch moves it forward in the right direction.
I’d appreciate it if people can test and provide feedback.

I’ve tested on amd64 under VirtualBox and GNU Configure proved to be an
excellent stress test and with this patch, running this on an ntfs-3g
mounted file system no longer causes fuse to hang.

FUSE Changes

1. Completed implementation of permission handling. FUSE now more
   closely behaves as if -o default_permissions was specified as a
   mount option.
2. FUSE now blocks indefinitely when waiting for the file system to
   respond. Support for Ctrl+C is planned.
3. More optional debug output for better tracing of VFS system calls
4. Minor cleanups

fuselib Changes
===
1. Improved option parsing so more mount options are now supported
2. Improved option parsing so that comma separated options are now
   supported
3. Unlinking or renaming an open file no longer causes the kernel to
   get sick
4. Support for -d option so fuse does not demonise


-- 
Helg xx...@msn.com


fuse.diff
Description: Binary data


libfuse.diff
Description: Binary data

Re: [patch] panic on boot when compiling kernel with option VFSLCKDEBUG

[Helg]

 While troubleshooting a deadlock problem I compiled the kernel with
 option VFSLCKDEBUG and found that the if statement in the assert macro
 is missing braces. The includes seem to have changed since 5.6 as well
 as it was missing the declaration for the panic function.
 
 
 Index: kern/vfs_vops.c
 ===
 RCS file: /cvs/src/sys/kern/vfs_vops.c,v
 retrieving revision 1.9
 diff -u -p -r1.9 vfs_vops.c
 --- kern/vfs_vops.c 13 Aug 2013 05:52:24 -  1.9
 +++ kern/vfs_vops.c 16 Jan 2015 23:30:59 -
 @@ -48,10 +48,12 @@
  #include sys/unistd.h
  
  #ifdef VFSLCKDEBUG
 +#include sys/systm.h
  #define ASSERT_VP_ISLOCKED(vp) do { \
 -if (((vp)-v_flag  VLOCKSWORK)  !VOP_ISLOCKED(vp))   \
 +if (((vp)-v_flag  VLOCKSWORK)  !VOP_ISLOCKED(vp)) { \
  VOP_PRINT(vp);  \
  panic(vp not locked); \
 +}   \
  } while (0)
  #else
  #define ASSERT_VP_ISLOCKED(vp)  /* nothing */
 
 
 -- 
 Helg xx...@msn.com
 

Anyone?

[patch] panic on boot when compiling kernel with option VFSLCKDEBUG

[Helg]

While troubleshooting a deadlock problem I compiled the kernel with
option VFSLCKDEBUG and found that the if statement in the assert macro
is missing braces. The includes seem to have changed since 5.6 as well
as it was missing the declaration for the panic function.


Index: kern/vfs_vops.c
===
RCS file: /cvs/src/sys/kern/vfs_vops.c,v
retrieving revision 1.9
diff -u -p -r1.9 vfs_vops.c
--- kern/vfs_vops.c 13 Aug 2013 05:52:24 -  1.9
+++ kern/vfs_vops.c 16 Jan 2015 23:30:59 -
@@ -48,10 +48,12 @@
 #include sys/unistd.h
 
 #ifdef VFSLCKDEBUG
+#include sys/systm.h
 #define ASSERT_VP_ISLOCKED(vp) do { \
-if (((vp)-v_flag  VLOCKSWORK)  !VOP_ISLOCKED(vp))   \
+if (((vp)-v_flag  VLOCKSWORK)  !VOP_ISLOCKED(vp)) { \
 VOP_PRINT(vp);  \
 panic(vp not locked); \
+}   \
 } while (0)
 #else
 #define ASSERT_VP_ISLOCKED(vp)  /* nothing */


-- 
Helg xx...@msn.com

Re: Kernel does not compile with option LOCKDEBUG

[Helg]

On Sat, 10 Jan 2015 19:39:19 -0800
Philip Guenther guent...@gmail.com wrote:

 On Mon, 5 Jan 2015, Helg wrote:
  The man page for LOCK(9) says that if the kernel option LOCKDEBUG is 
  enabled, additional facilities are provided to assist in determining 
  deadlock occurrences.
  
  I created a copy of /sys/arch/amd64/conf/GENERIC and added option 
  LOCKDEBUG. Executing config and then make clean  make results in 
  warnings like:
  
  implicit declaration of function '__mp_lock'
  
  Adding includes for sys/mplock.h in the offending files resolves the 
  problem but this just doesn't seem right.
  
  Does anyone have any suggestions?
 
 Yeah, don't use it.  It used to be a lot more when there were still traces 
 of the original simplelocks in the tree, but the code to do more with 
 LOCKDEBUG was ripped out as those no-ops were felt to be more misleading 
 than helpful.
 
 It's dead, Jim, let's bury LOCKDEBUG.
 
 oks?
 
 
 Philip
 
 Index: sys/arch/i386/i386/trap.c
 ===
 RCS file: /cvs/src/sys/arch/i386/i386/trap.c,v
 retrieving revision 1.119
 diff -u -p -r1.119 trap.c
 --- sys/arch/i386/i386/trap.c 2 Dec 2014 18:13:10 -   1.119
 +++ sys/arch/i386/i386/trap.c 11 Jan 2015 03:33:48 -
 @@ -381,11 +381,6 @@ trap(struct trapframe *frame)
   case T_PAGEFLT: /* allow page faults in kernel mode */
   if (p == 0 || p-p_addr == 0)
   goto we_re_toast;
 -#ifdef LOCKDEBUG
 - /* If we page-fault while in scheduler, we're doomed. */
 - if (__mp_lock_held(sched_lock))
 - goto we_re_toast;
 -#endif
  
   pcb = p-p_addr-u_pcb;
  #if 0
 Index: sys/sys/sched.h
 ===
 RCS file: /cvs/src/sys/sys/sched.h,v
 retrieving revision 1.37
 diff -u -p -r1.37 sched.h
 --- sys/sys/sched.h   17 Oct 2014 01:51:39 -  1.37
 +++ sys/sys/sched.h   11 Jan 2015 03:33:48 -
 @@ -183,7 +183,7 @@ void remrunqueue(struct proc *);
   yield();\
  } while (0)
  
 -#if defined(MULTIPROCESSOR) || defined(LOCKDEBUG)
 +#if defined(MULTIPROCESSOR)
  #include sys/lock.h
  
  /*
 @@ -215,7 +215,7 @@ do {  
 \
   splx(s);\
  } while (/* CONSTCOND */ 0)
  
 -#else /* ! MULTIPROCESSOR || LOCKDEBUG */
 +#else /* ! MULTIPROCESSOR */
  
  #define  SCHED_ASSERT_LOCKED()   splassert(IPL_SCHED);
  #define  SCHED_ASSERT_UNLOCKED() /* nothing */
 @@ -225,7 +225,7 @@ do {  
 \
  #define  SCHED_LOCK(s)   s = splsched()
  #define  SCHED_UNLOCK(s) splx(s)
  
 -#endif /* MULTIPROCESSOR || LOCKDEBUG */
 +#endif /* MULTIPROCESSOR */
  
  #endif   /* _KERNEL */
  #endif   /* _SYS_SCHED_H_ */
 Index: share/man/man9/lock.9
 ===
 RCS file: /cvs/src/share/man/man9/lock.9,v
 retrieving revision 1.22
 diff -u -p -r1.22 lock.9
 --- share/man/man9/lock.9 9 Jul 2014 14:16:10 -   1.22
 +++ share/man/man9/lock.9 11 Jan 2015 03:33:48 -
 @@ -57,10 +57,6 @@ single process.
  It also allows upgrading a shared-access lock to an
  exclusive-access lock, as well as downgrading an exclusive-access lock
  to a shared-access lock.
 -.Pp
 -If the kernel option LOCKDEBUG is enabled, additional facilities
 -are provided to record additional lock information.
 -These facilities are provided to assist in determining deadlock occurrences.
  .Sh FUNCTIONS
  The functions which operate on locks are:
  .Bl -tag -width Ds

There's another documentation bug on this man page. The flags parameter
to lockinit can only be 0. If you set it to anything other than this
you end up with a failed KASSERT.

-- 
Helg xx...@msn.com

Re: Kernel does not compile with option LOCKDEBUG

[Helg]

On Sat, 10 Jan 2015 19:39:19 -0800
Philip Guenther guent...@gmail.com wrote:

 On Mon, 5 Jan 2015, Helg wrote:
  The man page for LOCK(9) says that if the kernel option LOCKDEBUG is 
  enabled, additional facilities are provided to assist in determining 
  deadlock occurrences.
  
  I created a copy of /sys/arch/amd64/conf/GENERIC and added option 
  LOCKDEBUG. Executing config and then make clean  make results in 
  warnings like:
  
  implicit declaration of function '__mp_lock'
  
  Adding includes for sys/mplock.h in the offending files resolves the 
  problem but this just doesn't seem right.
  
  Does anyone have any suggestions?
 
 Yeah, don't use it.  It used to be a lot more when there were still traces 
 of the original simplelocks in the tree, but the code to do more with 
 LOCKDEBUG was ripped out as those no-ops were felt to be more misleading 
 than helpful.

Thanks. I figured as much when I started to look deeper into the
implementation of lockinit and lockmgr. These are now just legacy
wrappers for rwlock functions. Removing LOCKDEBUG makes sense to me.

 
 It's dead, Jim, let's bury LOCKDEBUG.
 
 oks?
 
 
 Philip
 
 Index: sys/arch/i386/i386/trap.c
 ===
 RCS file: /cvs/src/sys/arch/i386/i386/trap.c,v
 retrieving revision 1.119
 diff -u -p -r1.119 trap.c
 --- sys/arch/i386/i386/trap.c 2 Dec 2014 18:13:10 -   1.119
 +++ sys/arch/i386/i386/trap.c 11 Jan 2015 03:33:48 -
 @@ -381,11 +381,6 @@ trap(struct trapframe *frame)
   case T_PAGEFLT: /* allow page faults in kernel mode */
   if (p == 0 || p-p_addr == 0)
   goto we_re_toast;
 -#ifdef LOCKDEBUG
 - /* If we page-fault while in scheduler, we're doomed. */
 - if (__mp_lock_held(sched_lock))
 - goto we_re_toast;
 -#endif
  
   pcb = p-p_addr-u_pcb;
  #if 0
 Index: sys/sys/sched.h
 ===
 RCS file: /cvs/src/sys/sys/sched.h,v
 retrieving revision 1.37
 diff -u -p -r1.37 sched.h
 --- sys/sys/sched.h   17 Oct 2014 01:51:39 -  1.37
 +++ sys/sys/sched.h   11 Jan 2015 03:33:48 -
 @@ -183,7 +183,7 @@ void remrunqueue(struct proc *);
   yield();\
  } while (0)
  
 -#if defined(MULTIPROCESSOR) || defined(LOCKDEBUG)
 +#if defined(MULTIPROCESSOR)
  #include sys/lock.h
  
  /*
 @@ -215,7 +215,7 @@ do {  
 \
   splx(s);\
  } while (/* CONSTCOND */ 0)
  
 -#else /* ! MULTIPROCESSOR || LOCKDEBUG */
 +#else /* ! MULTIPROCESSOR */
  
  #define  SCHED_ASSERT_LOCKED()   splassert(IPL_SCHED);
  #define  SCHED_ASSERT_UNLOCKED() /* nothing */
 @@ -225,7 +225,7 @@ do {  
 \
  #define  SCHED_LOCK(s)   s = splsched()
  #define  SCHED_UNLOCK(s) splx(s)
  
 -#endif /* MULTIPROCESSOR || LOCKDEBUG */
 +#endif /* MULTIPROCESSOR */
  
  #endif   /* _KERNEL */
  #endif   /* _SYS_SCHED_H_ */
 Index: share/man/man9/lock.9
 ===
 RCS file: /cvs/src/share/man/man9/lock.9,v
 retrieving revision 1.22
 diff -u -p -r1.22 lock.9
 --- share/man/man9/lock.9 9 Jul 2014 14:16:10 -   1.22
 +++ share/man/man9/lock.9 11 Jan 2015 03:33:48 -
 @@ -57,10 +57,6 @@ single process.
  It also allows upgrading a shared-access lock to an
  exclusive-access lock, as well as downgrading an exclusive-access lock
  to a shared-access lock.
 -.Pp
 -If the kernel option LOCKDEBUG is enabled, additional facilities
 -are provided to record additional lock information.
 -These facilities are provided to assist in determining deadlock occurrences.
  .Sh FUNCTIONS
  The functions which operate on locks are:
  .Bl -tag -width Ds


-- 
Helg xx...@msn.com

Kernel does not compile with option LOCKDEBUG

[Helg]

Hi All,

The man page for LOCK(9) says that if the kernel option LOCKDEBUG is enabled, 
additional facilities are provided to assist in determining deadlock 
occurrences.

I created a copy of /sys/arch/amd64/conf/GENERIC and added option LOCKDEBUG. 
Executing config and then make clean  make results in warnings like:

implicit declaration of function '__mp_lock'

Adding includes for sys/mplock.h in the offending files resolves the problem 
but this just doesn't seem right.

Does anyone have any suggestions?

-- 
Helg

[PATCH] fuse file permissions

[Helg]

, cred, p
+   goto out;
 
if (vap-va_mode != (mode_t)VNOVAL) {
if (vp-v_mount-mnt_flag  MNT_RDONLY) {
error = EROFS;
goto out;
}
+
+   /* Only the owner or root can chmod */
+   if (cred-cr_uid != vattr.va_uid 
+   (error = suser_ucred(cred)))
+   goto out;
+
fbuf-fb_vattr.va_mode = vap-va_mode  ALLPERMS;
io-fi_flags |= FUSE_FATTR_MODE;
}
@@ -511,10 +550,12 @@ fusefs_setattr(void *v)
goto out;
}
 
+/* This is already checked above. 
if (io-fi_flags  FUSE_FATTR_SIZE  vp-v_type == VDIR) {
error = EISDIR;
goto out;
}
+*/
 
error = fb_queue(fmp-dev, fbuf);
 
Index: fusebuf.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fusebuf.c,v
retrieving revision 1.10
diff -u -p -r1.10 fusebuf.c
--- fusebuf.c   3 Dec 2014 23:00:49 -   1.10
+++ fusebuf.c   27 Dec 2014 08:54:22 -
@@ -55,7 +55,10 @@ fb_queue(dev_t dev, struct fusebuf *fbuf
 
fuse_device_queue_fbuf(dev, fbuf);
 
-   if ((error = tsleep(fbuf, PWAIT, fuse msg, TSLEEP_TIMEOUT * hz))) {
+   while ((error = tsleep(fbuf, PWAIT, fuse msg, 
+   TSLEEP_TIMEOUT * hz)) == EWOULDBLOCK);
+
+   if (error) {
fuse_device_cleanup(dev, fbuf);
return (error);
}


-- 
Helg

[PATCH] fuse mount realpath

[Helg]

This patch forces fuse to mount a client filesystem using the realpath
when a relative path is specified for the mount point. Without this, to
unmount a filesystem you need to change directory to the same folder
you where in when you mounted it. 

-- 
Helg xx...@msn.com


Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.21
diff -u -p -r1.21 fuse.c
--- fuse.c  24 Mar 2014 07:24:32 -  1.21
+++ fuse.c  18 May 2014 12:08:27 -
@@ -148,22 +148,27 @@ fuse_mount(const char *dir, unused struc
struct fusefs_args fargs;
struct fuse_chan *fc;
const char *errcause;
+   char real_dir[PATH_MAX];
 
fc = calloc(1, sizeof(*fc));
if (fc == NULL)
return (NULL);
 
-   fc-dir = strdup(dir);
-   if (fc-dir == NULL)
+   if (dir == NULL)
goto bad;
 
+   if (realpath(dir, real_dir) == NULL)
+   goto bad;
+
+   fc-dir = strdup(real_dir);
+
if ((fc-fd = open(/dev/fuse0, O_RDWR)) == -1) {
perror(__func__);
goto bad;
}
 
fargs.fd = fc-fd;
-   if (mount(MOUNT_FUSEFS, dir, 0, fargs)) {
+   if (mount(MOUNT_FUSEFS, real_dir, 0, fargs)) {
switch (errno) {
case EMFILE:
errcause = mount table full;

[Patch fuse] fusefs_link return code

[Helg]

This patch fixes a bug where fusefs_link does not return an error on
subsequent invocations if a fuse filesystem does not implement hard
links.

As an aside, returning ENOSYS in this case is contrary to the link(2)
man page (and different again to the Open Group) but consistent with
the fuse implementation on Linux.

-- 
Helg xx...@msn.com


Index: fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.16
diff -u -p -r1.16 fuse_vnops.c
--- fuse_vnops.c18 Mar 2014 08:51:53 -  1.16
+++ fuse_vnops.c4 May 2014 06:32:50 -
@@ -540,8 +540,10 @@ fusefs_link(void *v)
dip = VTOI(dvp);
fmp = (struct fusefs_mnt *)ip-ufs_ino.i_ump;
 
-   if (!fmp-sess_init || (fmp-undef_op  UNDEF_LINK))
+   if (!fmp-sess_init || (fmp-undef_op  UNDEF_LINK)) {
+   error = ENOSYS;
goto out1;
+   }
 
fbuf = fb_setup(cnp-cn_namelen + 1, dip-ufs_ino.i_number,
FBT_LINK, p);

[Patch fuselib] Support 255 character file names

[Helg]

I've been doing some testing of fuse and discovered a small bug where it only 
allows file names up to 254 characters due to not taking the NULL terminator 
into consideration when allocating structures.

-- 
Helg xx...@msn.com



Index: dict.c
===
RCS file: /cvs/src/lib/libfuse/dict.c,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 dict.c
--- dict.c  3 Jun 2013 16:00:50 -   1.1
+++ dict.c  27 Apr 2014 00:30:21 -
@@ -26,7 +26,7 @@
 #defineMAX_DICTKEY_SIZENAME_MAX
 struct dictentry {
SPLAY_ENTRY(dictentry)  entry;
-   charkey[MAX_DICTKEY_SIZE];
+   charkey[MAX_DICTKEY_SIZE + 1];
void   *data;
 };
 
Index: fuse_private.h
===
RCS file: /cvs/src/lib/libfuse/fuse_private.h,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 fuse_private.h
--- fuse_private.h  3 Dec 2013 09:59:40 -   1.9
+++ fuse_private.h  27 Apr 2014 00:30:22 -
@@ -34,7 +34,7 @@ struct fuse_vnode {
ino_t ino;
ino_t parent;
 
-   char path[NAME_MAX];
+   char path[NAME_MAX + 1];
struct fuse_dirhandle *fd;
 
SIMPLEQ_ENTRY(fuse_vnode) node; /* for dict */
Index: fuse_subr.c
===
RCS file: /cvs/src/lib/libfuse/fuse_subr.c,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 fuse_subr.c
--- fuse_subr.c 5 Feb 2014 20:13:58 -   1.7
+++ fuse_subr.c 27 Apr 2014 00:30:22 -
@@ -35,8 +35,7 @@ alloc_vn(struct fuse *f, const char *pat
 
vn-ino = ino;
vn-parent = parent;
-   strncpy(vn-path, path, NAME_MAX);
-   vn-path[NAME_MAX - 1] =  '\0';
+   strlcpy(vn-path, path, sizeof(vn-path));
if (ino == (ino_t)-1) {
f-max_ino++;
vn-ino = f-max_ino;