Fix readdir() bug when a non-zero offset is specified in filler (#269)

The bug occurs when a filesystem client reads a directory until the end,
seeks using seekdir() to some valid non-zero position and calls
readdir(). A valid 'struct dirent *' is expected, but NULL is returned
instead. Pseudocode demonstrating the bug:

DIR *dp = opendir("some_dir");
struct dirent *de = readdir(dp);

/* Get offset of the second entry */
long offset = telldir(dp);

/* Read directory until the end */
while (de)
	de = readdir(de);

seekdir(dp, offset);
de = readdir(dp);
/* de must contain the second entry, but NULL is returned instead */

The reason of the bug is that when the end of directory is reached, the
kernel calls FUSE_READDIR op with an offset at the end of directory, so
the filesystem's .readdir callback never calls the filler function, and
we end up with dh->filled set to 1. After seekdir(), FUSE_READDIR is
called again with a new offset, but this time the filesystem's .readdir
callback is never called, and an empty reply is returned.

Fix by setting dh->filled to 1 only when zero offsets are given to
filler function.

diff --git a/ChangeLog.rst b/ChangeLog.rst
index cf5450e6fc855a6e668c9dfb434712882c589608..9ba0263a49b1badf266b6c7a5fa87f41ca5405eb 100644 (file)
--- a/ChangeLog.rst
+++ b/ChangeLog.rst
@@ -1,3 +1,11 @@
+libfuse 3.2.5
+==========================
+
+* Added a test of `seekdir` to test_syscalls.
+* Fixed `readdir` bug when non-zero offsets are given to filler and the
+  filesystem client, after reading a whole directory, re-reads it from a
+  non-zero offset e. g. by calling `seekdir` followed by `readdir`.
+
 libfuse 3.2.4 (2018-07-11)
 ==========================
 

diff --git a/lib/fuse.c b/lib/fuse.c
index 334b9772466eaa8ba8139ccc8a7f82b0f5fe2a75..ea7d3b322e5de6b13f4c7be514b47f5ed0e16c15 100644 (file)
--- a/lib/fuse.c
+++ b/lib/fuse.c
@@ -3455,6 +3455,11 @@ static int fill_dir(void *dh_, const char *name, const struct stat *statp,
        if (off) {
                size_t newlen;
 
+               if (dh->filled) {
+                       dh->error = -EIO;
+                       return 1;
+               }
+
                if (dh->first) {
                        dh->error = -EIO;
                        return 1;
@@ -3463,7 +3468,6 @@ static int fill_dir(void *dh_, const char *name, const struct stat *statp,
                if (extend_contents(dh, dh->needlen) == -1)
                        return 1;
 
-               dh->filled = 0;
                newlen = dh->len +
                        fuse_add_direntry(dh->req, dh->contents + dh->len,
                                          dh->needlen - dh->len, name,
@@ -3473,10 +3477,8 @@ static int fill_dir(void *dh_, const char *name, const struct stat *statp,
 
                dh->len = newlen;
        } else {
-               if (!dh->filled) {
-                       dh->error = -EIO;
-                       return 1;
-               }
+               dh->filled = 1;
+
                if (fuse_add_direntry_to_dh(dh, name, &stbuf) == -1)
                        return 1;
        }
@@ -3526,6 +3528,11 @@ static int fill_dir_plus(void *dh_, const char *name, const struct stat *statp,
        if (off) {
                size_t newlen;
 
+               if (dh->filled) {
+                       dh->error = -EIO;
+                       return 1;
+               }
+
                if (dh->first) {
                        dh->error = -EIO;
                        return 1;
@@ -3533,7 +3540,6 @@ static int fill_dir_plus(void *dh_, const char *name, const struct stat *statp,
                if (extend_contents(dh, dh->needlen) == -1)
                        return 1;
 
-               dh->filled = 0;
                newlen = dh->len +
                        fuse_add_direntry_plus(dh->req, dh->contents + dh->len,
                                               dh->needlen - dh->len, name,
@@ -3542,10 +3548,8 @@ static int fill_dir_plus(void *dh_, const char *name, const struct stat *statp,
                        return 1;
                dh->len = newlen;
        } else {
-               if (!dh->filled) {
-                       dh->error = -EIO;
-                       return 1;
-               }
+               dh->filled = 1;
+
                if (fuse_add_direntry_to_dh(dh, name, &e.attr) == -1)
                        return 1;
        }
@@ -3588,7 +3592,7 @@ static int readdir_fill(struct fuse *f, fuse_req_t req, fuse_ino_t ino,
                dh->len = 0;
                dh->error = 0;
                dh->needlen = size;
-               dh->filled = 1;
+               dh->filled = 0;
                dh->req = req;
                fuse_prepare_interrupt(f, req, &d);
                err = fuse_fs_readdir(f->fs, path, dh, filler, off, fi, flags);

diff --git a/test/test_syscalls.c b/test/test_syscalls.c
index 38a37a1b37cc3f7ad49520e2c40aea524c0cdab2..4e1dc07befbf2117e3d290d6681cd954f3f8f8ad 100644 (file)
--- a/test/test_syscalls.c
+++ b/test/test_syscalls.c
@@ -28,6 +28,7 @@ static char testname[256];
 static char testdata[] = "abcdefghijklmnopqrstuvwxyz";
 static char testdata2[] = "1234567890-=qwertyuiop[]\asdfghjkl;'zxcvbnm,./";
 static const char *testdir_files[] = { "f1", "f2", NULL};
+static long seekdir_offsets[4];
 static char zerodata[4096];
 static int testdatalen = sizeof(testdata) - 1;
 static int testdata2len = sizeof(testdata2) - 1;
@@ -87,6 +88,8 @@ static void __start_test(const char *fmt, ...)
 #define PERROR(msg) test_perror(__FUNCTION__, msg)
 #define ERROR(msg, args...) test_error(__FUNCTION__, msg, ##args)
 
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
 static int check_size(const char *path, int len)
 {
        struct stat stbuf;
@@ -651,6 +654,63 @@ static int test_ftruncate(int len, int mode)
        return 0;
 }
 
+static int test_seekdir(void)
+{
+       int i;
+       int res;
+       DIR *dp;
+       struct dirent *de;
+
+       start_test("seekdir");
+       res = create_dir(testdir, testdir_files);
+       if (res == -1)
+               return res;
+
+       dp = opendir(testdir);
+       if (dp == NULL) {
+               PERROR("opendir");
+               return -1;
+       }
+
+       /* Remember dir offsets */
+       for (i = 0; i < ARRAY_SIZE(seekdir_offsets); i++) {
+               seekdir_offsets[i] = telldir(dp);
+               errno = 0;
+               de = readdir(dp);
+               if (de == NULL) {
+                       if (errno) {
+                               PERROR("readdir");
+                               goto fail;
+                       }
+                       break;
+               }
+       }
+
+       /* Walk until the end of directory */
+       while (de)
+               de = readdir(dp);
+
+       /* Start from the last valid dir offset and seek backwards */
+       for (i--; i >= 0; i--) {
+               seekdir(dp, seekdir_offsets[i]);
+               de = readdir(dp);
+               if (de == NULL) {
+                       ERROR("Unexpected end of directory after seekdir()");
+                       goto fail;
+               }
+       }
+
+       closedir(dp);
+       res = cleanup_dir(testdir, testdir_files, 0);
+       if (!res)
+               success();
+       return res;
+fail:
+       closedir(dp);
+       cleanup_dir(testdir, testdir_files, 1);
+       return -1;
+}
+
 static int test_utime(void)
 {
        struct utimbuf utm;
@@ -1677,6 +1737,7 @@ int main(int argc, char *argv[])
        err += test_rename_file();
        err += test_rename_dir();
        err += test_rename_dir_loop();
+       err += test_seekdir();
        err += test_utime();
        err += test_truncate(0);
        err += test_truncate(testdatalen / 2);