On Wed, 18 Oct 2017 10:47:59 +0200
Martin Pieuchot <[email protected]> wrote:
> On 16/10/17(Mon) 14:37, Helg Bredow wrote:
> > [...]
> > Patch is below.
>
> Do you mind explaining the problem and the solution you implemented?
>
There is a regression introduced in 6.2 that can cause a call to getcwd(3) to
result in an endless loop. fusefs_readdir() has a TODO that returns success
even if a directory is not open, so getcwd(3) keeps calling it to fetch more
dirents but none ever get supplied. This patch opens and closes the directory
if it's not already open. The check for an invalid file handle was inside the
loop but I've moved it outside since it's not going to become invalid once it's
been opened.
This doesn't resolve the problem however. This is because ifuse_ops_readdir()
does not support being called across multiple calls to VOP_READDIR(9). This
occurs if the buffer is too small to contain all the entries and getcwd(3) only
supplies a 512 byte buffer. If the current directory is not returned in the
first call to VOP_READDIR(9) then getcwd(3) will continue to call it but the
eofflag is never set. ifuse_ops_readdir() now correctly starts scanning at the
next dirent even across multiple VOP_READDIR(9) calls.
In addition, if the entries don't evenly fit into the buffer then
fusefs_readdir() would prematurely detect that the entire directory had been
scanned and set the eofflag. This fix handles this condition so the entire
directory is always scanned.
ifuse_fill_readdir() returns the ino of the file system instead of its own ino,
which it returns from other calls such as stat(2). getcwd(3) compares these
inos when scanning the directory and will never find a match since they are
sourced from different file systems. fuse has the use_ino option to allow a
file system to use its own inos but this is not implemented in OpenBSD yet.
This patch forces the fuse ino to be used in ifuse_fill_readdir(). This is
still only paritally implemented though, it will only set a valid ino if it can
find the file or directory in the name cache. This is sufficient for getcwd(3)
since the parent directory will always be in the cache.
Thanks for your feedback mpi@; I've made the changes you suggested. It's safe
to store the fuse struct across calls since this is returned from fuse_setup()
and never changes. Without it, I'm not sure how I would implement a solution
since you need this in the filler function to access the name dictionary.
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 18 Oct 2017 13:24:53 -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 18 Oct 2017 13:24:53 -0000
@@ -217,17 +217,20 @@ 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 || fbuf->fb_len + len > fd->size) {
fd->full = 1;
return (0);
}
@@ -242,13 +245,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 +306,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 +330,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 +358,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 18 Oct 2017 13:25:04 -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);
- 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 */
+ 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);
return (error);
}
+ diropen = 1;
+ }
+
+ while (uio->uio_resid > 0) {
+ fbuf = fb_setup(0, ip->ufs_ino.i_number, FBT_READDIR, p);
+
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,13 @@ fusefs_readdir(void *v)
error = fb_queue(fmp->dev, fbuf);
if (error) {
+ /*
+ * dirent was larger than residual space left in
+ * buffer.
+ */
+ if (error == ENOBUFS && fbuf->fb_len == 0)
+ error = 0;
+
fb_delete(fbuf);
break;
}
@@ -731,6 +742,9 @@ 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);
return (error);
}
