Prevent recreating dir nodes in readdir

The readdir implementation would generate new nodes for every directory
it encountered, even if they had seen before, returning new inodes each
time.  Other than this having a performance impact, this is just
incorrect and I'm not sure how this bug has survived for so long.

In fact, the only reason I noticed this is because I tried to build
GNU tar on top of sandboxfs and its configure script would fail when
run as root.  Essentially, the test was doing a rename of a directory
inside a directory, and that somehow caused the pwd to become invalid
when performing a readdir on the parent directory.  The issue is easier
to reproduce on Linux though.

Fixes #92.

NEWS.md

@@ -4,6 +4,9 @@
 
 **Released 2019-10-24.**
 
+* Issue #92: Fixed a bug in `readdir` where we would regenerate inodes for
+  directory entries, causing later confusion when trying to access those directories.
+
 * Fixed the definition of `--input` and `--output` to require an argument,
   which makes `--foo bar` and `--foo=bar` equivalent.  This can be thought to
   break backwards compatibility but, in reality, it does not.  The previous

integration/read_write_test.go

@@ -262,6 +262,46 @@ func TestReadWrite_WriteOnDeletedAndDuppedFd(t *testing.T) {
 	}
 }
 
+func TestReadWrite_InodesArePreservedDuringReaddir(t *testing.T) {
+	state := utils.MountSetup(t, "--mapping=rw:/:%ROOT%")
+	defer state.TearDown(t)
+
+	// inodeOf obtains the inode number of a file.
+	inodeOf := func(path string) uint64 {
+		fileInfo, err := os.Lstat(path)
+		if err != nil {
+			t.Fatalf("Failed to get inode number of %s: %v", path, err)
+		}
+		return fileInfo.Sys().(*syscall.Stat_t).Ino
+	}
+
+	utils.MustMkdirAll(t, state.MountPath("dir"), 0755)
+	utils.MustWriteFile(t, state.MountPath("file"), 0644, "")
+
+	for _, name := range []string{"dir", "file"} {
+		previous := inodeOf(state.MountPath(name))
+		if found, err := existsViaReaddir(state.MountPath(), name); err != nil || !found {
+			t.Fatalf("Cannot find entry %s: %v", name, err)
+		}
+
+		// Rename the entry to force the kernel to "rediscover" it during the next readdir.
+		// Otherwise, we might not trigger bugs with inode preservation, and the behavior of
+		// this depends on the platform.
+		newName := name + "-new"
+		if err := os.Rename(state.MountPath(name), state.MountPath(newName)); err != nil {
+			t.Fatalf("Cannot rename %s: %v", name, err)
+		}
+
+		if found, err := existsViaReaddir(state.MountPath(), newName); err != nil || !found {
+			t.Fatalf("Cannot find entry %s: %v", newName, err)
+		}
+		now := inodeOf(state.MountPath(newName))
+		if previous != now {
+			t.Errorf("Inode number was not stable for %s; got %v, want %v", name, now, previous)
+		}
+	}
+}
+
 func TestReadWrite_InodeReassignedAfterRecreation(t *testing.T) {
 	state := utils.MountSetup(t, "--mapping=rw:/:%ROOT%")
 	defer state.TearDown(t)

src/nodes/dir.rs

@@ -126,11 +126,18 @@ impl OpenDir {
             }
 
             if let Some(dirent) = state.children.get(&name) {
-                if dirent.explicit_mapping {
-                    // Found an on-disk entry that also corresponds to an explicit mapping by the
-                    // user.  Nothing to do: we already handled this case above.
-                    continue;
+                // Found a previously-known on-disk entry.  Must return it "as is" (even if its
+                // type might have changed) because, if we called into `cache.get_or_create` below,
+                // we might recreate the node unintentionally.  Note that mappings were handled
+                // earlier, so only handle the non-mapping case here.
+                if !dirent.explicit_mapping {
+                    reply.push(ReplyEntry {
+                        inode: dirent.node.inode(),
+                        fs_type: dirent.node.file_type_cached(),
+                        name: name.clone(),
+                    });
                 }
+                continue;
             }
 
             let path = state.underlying_path.as_ref().unwrap().join(&name);