Fix infinite looping behavior with readdir.

Also adds partial . and .. support, and implements ENOTDIR error.

The source of this problem was due to the off-by-one interface expected
by readdir. Each directory entry contains a pointer to next, so it
cannot be emitted to the user until the next entry is actually read.

Returning the current position + 1 is insufficient, because the user
will just retry cur + 1 as the start position to see if they've reached
the end of the directory.

Since directory entries are keyed by a 64-bit hash in bcachefs, the
result was that the user would call readdir over and over with what it
believed the next pointer to be until "cur + 1" reached some enormous
64-bit value related to the hash for the "lost+found" entry.

diff --git a/cmd_fusemount.c b/cmd_fusemount.c
index a9d6305bf6a44733cfeda7c16c0001fe5829d500..a7c558895b4199a2cb4aaa70758e485caa838d6f 100644 (file)
--- a/cmd_fusemount.c
+++ b/cmd_fusemount.c
@@ -360,38 +360,98 @@ static void bcachefs_fuse_opendir(fuse_req_t req, fuse_ino_t inum,
 }
 #endif
 
+struct fuse_dir_entry {
+       u64             ino;
+       unsigned        type;
+       char            name[0];
+};
+
 struct fuse_dir_context {
        struct dir_context      ctx;
        fuse_req_t              req;
        char                    *buf;
        size_t                  bufsize;
+
+       struct fuse_dir_entry   *prev;
 };
 
-static int fuse_filldir(struct dir_context *_ctx,
-                       const char *name, int namelen,
-                       loff_t pos, u64 dir, unsigned type)
+static int fuse_send_dir_entry(struct fuse_dir_context *ctx, loff_t pos)
 {
-       struct fuse_dir_context *ctx =
-               container_of(_ctx, struct fuse_dir_context, ctx);
+       struct fuse_dir_entry *de = ctx->prev;
+       ctx->prev = NULL;
 
        struct stat statbuf = {
-               .st_ino         = map_root_ino(dir),
-               .st_mode        = type << 12,
+               .st_ino         = unmap_root_ino(de->ino),
+               .st_mode        = de->type << 12,
        };
 
-       size_t len = fuse_add_direntry(ctx->req,
-                                      ctx->buf,
-                                      ctx->bufsize,
-                                      name,
-                                      &statbuf,
-                                      pos + 1);
+       size_t len = fuse_add_direntry(ctx->req, ctx->buf, ctx->bufsize,
+                                      de->name, &statbuf, pos);
+
+       free(de);
 
        if (len > ctx->bufsize)
-               return 0;
+               return -EINVAL;
 
        ctx->buf        += len;
        ctx->bufsize    -= len;
-       return 1;
+
+       return 0;
+}
+
+static int fuse_filldir(struct dir_context *_ctx,
+                       const char *name, int namelen,
+                       loff_t pos, u64 ino, unsigned type)
+{
+       struct fuse_dir_context *ctx =
+               container_of(_ctx, struct fuse_dir_context, ctx);
+
+       fuse_log(FUSE_LOG_DEBUG, "fuse_filldir(ctx={.ctx={.pos=%llu}}, "
+                "name=%s, namelen=%d, pos=%lld, dir=%llu, type=%u)\n",
+                ctx->ctx.pos, name, namelen, pos, ino, type);
+
+       /*
+        * We have to emit directory entries after reading the next entry,
+        * because the previous entry contains a pointer to next.
+        */
+       if (ctx->prev) {
+               int ret = fuse_send_dir_entry(ctx, pos);
+               if (ret)
+                       return ret;
+       }
+
+       struct fuse_dir_entry *cur = malloc(sizeof *cur + namelen + 1);
+       cur->ino = ino;
+       cur->type = type;
+       memcpy(cur->name, name, namelen);
+       cur->name[namelen] = 0;
+
+       ctx->prev = cur;
+
+       return 0;
+}
+
+static bool handle_dots(struct fuse_dir_context *ctx, fuse_ino_t dir)
+{
+       int ret = 0;
+
+       if (ctx->ctx.pos == 0) {
+               ret = fuse_filldir(&ctx->ctx, ".", 1, ctx->ctx.pos,
+                                  unmap_root_ino(dir), DT_DIR);
+               if (ret < 0)
+                       return false;
+               ctx->ctx.pos = 1;
+       }
+
+       if (ctx->ctx.pos == 1) {
+               ret = fuse_filldir(&ctx->ctx, "..", 2, ctx->ctx.pos,
+                                  /*TODO: parent*/ 1, DT_DIR);
+               if (ret < 0)
+                       return false;
+               ctx->ctx.pos = 2;
+       }
+
+       return true;
 }
 
 static void bcachefs_fuse_readdir(fuse_req_t req, fuse_ino_t dir,
@@ -399,24 +459,58 @@ static void bcachefs_fuse_readdir(fuse_req_t req, fuse_ino_t dir,
                                  struct fuse_file_info *fi)
 {
        struct bch_fs *c = fuse_req_userdata(req);
-       char buf[4096];
+       struct bch_inode_unpacked bi;
+       char *buf = calloc(size, 1);
        struct fuse_dir_context ctx = {
                .ctx.actor      = fuse_filldir,
                .ctx.pos        = off,
                .req            = req,
                .buf            = buf,
-               .bufsize        = sizeof(buf),
+               .bufsize        = size,
        };
-       int ret;
+       int ret = 0;
+
+       fuse_log(FUSE_LOG_DEBUG, "bcachefs_fuse_readdir(dir=%llu, size=%zu, "
+                "off=%lld)\n", dir, size, off);
 
        dir = map_root_ino(dir);
 
+       ret = bch2_inode_find_by_inum(c, dir, &bi);
+       if (ret)
+               goto reply;
+
+       if (!S_ISDIR(bi.bi_mode)) {
+               ret = -ENOTDIR;
+               goto reply;
+       }
+
+       if (!handle_dots(&ctx, dir))
+               goto reply;
+
        ret = bch2_readdir(c, dir, &ctx.ctx);
+
+reply:
+       /*
+        * If we have something to send, the error above doesn't matter.
+        *
+        * Alternatively, if this send fails, but we previously sent something,
+        * then this is a success.
+        */
+       if (ctx.prev) {
+               ret = fuse_send_dir_entry(&ctx, ctx.ctx.pos);
+               if (ret && ctx.buf != buf)
+                       ret = 0;
+       }
+
        if (!ret) {
+               fuse_log(FUSE_LOG_DEBUG, "bcachefs_fuse_readdir reply %zd\n",
+                                       ctx.buf - buf);
                fuse_reply_buf(req, buf, ctx.buf - buf);
        } else {
                fuse_reply_err(req, -ret);
        }
+
+       free(buf);
 }
 
 #if 0