On Sun, 15 Oct 2017 14:22:06 +0000
Helg Bredow <[email protected]> wrote:

> On Fri, 13 Oct 2017 12:54:31 +0000
> Helg Bredow <[email protected]> wrote:
> 
> > On Wed, 11 Oct 2017 13:53:26 +0200
> > Martin Pieuchot <[email protected]> wrote:
> > 
> > > On 11/10/17(Wed) 11:25, Helg Bredow wrote:
> > > > I initially noticed this on a modified kernel that returns EBADF when
> > > > the fusefs_readdir() code you modified is encountered. 
> > > 
> > > Yes, there's a bug in the current code.  A functionality is not
> > > implemented and no error is returned.
> > > 
> > > What's the relevant part of ktrace/kdump with this modified kernel?
> > > 
> > > >                                                        I then
> > > > downloaded the latest snapshot to see if I could replicate it and
> > > > noticed that it "hangs" the kernel (due to an endless loop). This bug
> > > > is not present in 6.1.
> > > 
> > > You should be able to enter ddb(4) and use the new "kill" command to
> > > get out of the hang.
> > > 
> > > > I used ktrace to see what system calls are being made and narrowed it
> > > > down to getcwd(3). I've noticed that there is no call to VOP_OPEN in
> > > > vfs_getcwd.c. Is that the correct behaviour?
> > > 
> > > Yes it is.
> > > 
> > > > I've tried your patch and had to modify it a bit so it compiles but it
> > > > doesn't solve the problem. The fusefs_file_open() call succeeds.
> > > > However, any call to getcwd(3) now fails with "No such file or
> > > > directory". I tested by creating a small test program that just makes
> > > > this function call.
> > > 
> > > You need to figure out why vfs_getcwd_scandir() returns ENOENT.   What's
> > > failing inside fuse_readdir() and why?
> > > 
> > 
> > The reason is that ifuse_fill_readdir() sets d_fileno to the ino value 
> > returned by the file system but fuse otherwise uses its own generated ino. 
> > The Linux version of fuse has the use_ino and readdir_ino options to modify 
> > this behaviour. OpenBSD currently behaves as if the use_ino option was not 
> > set. i.e. don't use the file system ino values.
> > 
> > I've attempted to fix this by calling get_vn_by_name_and_parent() inside 
> > the filler function and it works some of the time but not others. Any 
> > suggestions?
> > 
> 
> Just in case anyone else is working on this. I've found the cause and am 
> working on a patch.
> 
> -- 
> Helg <[email protected]>

Patch is below.


Index: lib/libfuse/fuse.h
===================================================================
RCS file: /cvs/src/lib/libfuse/fuse.h,v
retrieving revision 1.12
diff -u -p -r1.12 fuse.h
--- lib/libfuse/fuse.h  20 Jan 2014 15:01:59 -0000      1.12
+++ lib/libfuse/fuse.h  16 Oct 2017 14:21:06 -0000
@@ -89,6 +89,7 @@ typedef int (*fuse_fill_dir_t)(void *, c
     off_t);
 
 typedef struct fuse_dirhandle {
+       struct fuse *fuse;
        fuse_fill_dir_t filler;
        void *buf;
        int filled;
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 -0000       1.26
+++ lib/libfuse/fuse_ops.c      16 Oct 2017 14:21:06 -0000
@@ -217,17 +217,23 @@ static int
 ifuse_fill_readdir(void *dh, const char *name, const struct stat *stbuf,
     off_t off)
 {
+       struct fuse *f;
        struct fuse_dirhandle *fd = dh;
+       struct fuse_vnode *v;
        struct fusebuf *fbuf;
        struct dirent *dir;
        uint32_t namelen;
        uint32_t len;
 
+       f = fd->fuse;
        fbuf = fd->buf;
        namelen = strnlen(name, MAXNAMLEN);
        len = GENERIC_DIRSIZ(namelen);
 
-       if (fd->full || (fbuf->fb_len + len > fd->size)) {
+       if (fd->full)
+               return (0);
+
+       if (fbuf->fb_len + len > fd->size) {
                fd->full = 1;
                return (0);
        }
@@ -242,13 +248,21 @@ ifuse_fill_readdir(void *dh, const char 
        if (off)
                fd->filled = 0;
 
-       if (stbuf) {
-               dir->d_fileno = stbuf->st_ino;
+       /* TODO Add support for use_ino and readdir_ino */
+       v = get_vn_by_name_and_parent(f, (uint8_t *)name, fbuf->fb_ino);
+       if (v == NULL) {
+               if (strcmp(name, ".") == 0)
+                       dir->d_fileno = fbuf->fb_ino;
+               else
+                       dir->d_fileno = 0xffffffff;
+       } else
+               dir->d_fileno = v->ino;
+
+       if (stbuf)
                dir->d_type = IFTODT(stbuf->st_mode);
-       } else {
-               dir->d_fileno = 0xffffffff;
+       else
                dir->d_type = DT_UNKNOWN;
-       }
+
        dir->d_reclen = len;
        dir->d_off = off + len;         /* XXX */
        strlcpy(dir->d_name, name, sizeof(dir->d_name));
@@ -295,7 +309,7 @@ ifuse_ops_readdir(struct fuse *f, struct
        ffi.fh = fbuf->fb_io_fd;
        offset = fbuf->fb_io_off;
        size = fbuf->fb_io_len;
-       startsave = 0;
+       startsave = offset;
 
        fbuf->fb_dat = calloc(1, size);
 
@@ -319,6 +333,8 @@ ifuse_ops_readdir(struct fuse *f, struct
                vn->fd->size = size;
                vn->fd->off = offset;
                vn->fd->idx = 0;
+               vn->fd->fuse = f;
+               vn->fd->start = offset;
                startsave = vn->fd->start;
 
                realname = build_realname(f, vn->ino);
@@ -345,7 +361,8 @@ ifuse_ops_readdir(struct fuse *f, struct
        if (fbuf->fb_err) {
                fbuf->fb_len = 0;
                vn->fd->filled = 1;
-       }
+       } else if (vn->fd->full && fbuf->fb_len == 0)
+               fbuf->fb_err = -ENOBUFS;
 
        if (fbuf->fb_len == 0)
                free(fbuf->fb_dat);
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.c        7 Sep 2016 17:53:35 -0000       1.33
+++ sys/miscfs/fuse/fuse_vnops.c        16 Oct 2017 14:21:17 -0000
@@ -680,7 +680,7 @@ fusefs_readdir(void *v)
        struct vnode *vp;
        struct proc *p;
        struct uio *uio;
-       int error = 0, eofflag = 0;
+       int error = 0, eofflag = 0, diropen = 0;
 
        vp = ap->a_vp;
        uio = ap->a_uio;
@@ -695,14 +695,18 @@ fusefs_readdir(void *v)
        if (uio->uio_resid < sizeof(struct dirent))
                return (EINVAL);
 
+       if (ip->fufh[FUFH_RDONLY].fh_type == FUFH_INVALID) {
+               error = fusefs_file_open(fmp, ip, FUFH_RDONLY, O_RDONLY, 1, p);
+               if (error) {
+                       fb_delete(fbuf);
+                       goto out;
+               }
+               diropen = 1;
+       }
+
        while (uio->uio_resid > 0) {
                fbuf = fb_setup(0, ip->ufs_ino.i_number, FBT_READDIR, p);
 
-               if (ip->fufh[FUFH_RDONLY].fh_type == FUFH_INVALID) {
-                       /* TODO open the file */
-                       fb_delete(fbuf);
-                       return (error);
-               }
                fbuf->fb_io_fd = ip->fufh[FUFH_RDONLY].fh_id;
                fbuf->fb_io_off = uio->uio_offset;
                fbuf->fb_io_len = MIN(uio->uio_resid, fmp->max_read);
@@ -710,6 +714,11 @@ fusefs_readdir(void *v)
                error = fb_queue(fmp->dev, fbuf);
 
                if (error) {
+                       /* Not a real error, dirent was larger than residual
+                          space left in buffer. Return as normal. */
+                       if (error == ENOBUFS && fbuf->fb_len == 0)
+                               error = 0;
+
                        fb_delete(fbuf);
                        break;
                }
@@ -732,6 +741,10 @@ fusefs_readdir(void *v)
        if (!error && ap->a_eofflag != NULL)
                *ap->a_eofflag = eofflag;
 
+       if (diropen)
+               fusefs_file_close(fmp, ip, FUFH_RDONLY, O_RDONLY, 1, p);
+
+out:
        return (error);
 }
 


Reply via email to