Call lseek(0, SEEK_CUR) unconditionally in runsc fsgofer's Readdir(offset=0).

9P2000.L is silent as to how readdir RPCs interact with directory mutation. The
most performant option is for Treaddir with offset=0 to restart iteration,
avoiding needing to walk+open+clunk a new directory fid between invocations of
getdents64(2), and the VFS2 gofer client assumes this is the case. Make this
actually true for the runsc fsgofer.

Fixes #3344, #3345, #3355

PiperOrigin-RevId: 324090384

6 files changed, 114 insertions, 6 deletions

diff --git a/images/basic/hostoverlaytest/Dockerfile b/images/basic/hostoverlaytest/Dockerfile
index d83439e9c..6cef1a542 100644
--- a/images/basic/hostoverlaytest/Dockerfile
+++ b/images/basic/hostoverlaytest/Dockerfile
@@ -4,4 +4,5 @@ WORKDIR /root
 COPY . .
 
 RUN apt-get update && apt-get install -y gcc
-RUN gcc -O2 -o test test.c
+RUN gcc -O2 -o test_copy_up test_copy_up.c
+RUN gcc -O2 -o test_rewinddir test_rewinddir.c
diff --git a/images/basic/hostoverlaytest/testfile.txt b/images/basic/hostoverlaytest/copy_up_testfile.txt
index e4188c841..e4188c841 100644
--- a/images/basic/hostoverlaytest/testfile.txt
+++ b/images/basic/hostoverlaytest/copy_up_testfile.txt
diff --git a/images/basic/hostoverlaytest/test.c b/images/basic/hostoverlaytest/test_copy_up.c
index 088f90746..010b261dc 100644
--- a/images/basic/hostoverlaytest/test.c
+++ b/images/basic/hostoverlaytest/test_copy_up.c
@@ -6,7 +6,7 @@
 #include <unistd.h>
 
 int main(int argc, char** argv) {
-  const char kTestFilePath[] = "testfile.txt";
+  const char kTestFilePath[] = "copy_up_testfile.txt";
   const char kOldFileData[] = "old data\n";
   const char kNewFileData[] = "new data\n";
   const size_t kPageSize = sysconf(_SC_PAGE_SIZE);
diff --git a/images/basic/hostoverlaytest/test_rewinddir.c b/images/basic/hostoverlaytest/test_rewinddir.c
new file mode 100644
index 000000000..f1a4085e1
--- /dev/null
+++ b/images/basic/hostoverlaytest/test_rewinddir.c
@@ -0,0 +1,78 @@
+#include <dirent.h>
+#include <err.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+int main(int argc, char** argv) {
+  const char kDirPath[] = "rewinddir_test_dir";
+  const char kFileBasename[] = "rewinddir_test_file";
+
+  // Create the test directory.
+  if (mkdir(kDirPath, 0755) < 0) {
+    err(1, "mkdir(%s)", kDirPath);
+  }
+
+  // The test directory should initially be empty.
+  DIR* dir = opendir(kDirPath);
+  if (!dir) {
+    err(1, "opendir(%s)", kDirPath);
+  }
+  int failed = 0;
+  while (1) {
+    errno = 0;
+    struct dirent* d = readdir(dir);
+    if (!d) {
+      if (errno != 0) {
+        err(1, "readdir");
+      }
+      break;
+    }
+    if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0) {
+      warnx("unexpected file %s in new directory", d->d_name);
+      failed = 1;
+    }
+  }
+
+  // Create a file in the test directory.
+  char* file_path = malloc(strlen(kDirPath) + 1 + strlen(kFileBasename));
+  if (!file_path) {
+    errx(1, "malloc");
+  }
+  strcpy(file_path, kDirPath);
+  file_path[strlen(kDirPath)] = '/';
+  strcpy(file_path + strlen(kDirPath) + 1, kFileBasename);
+  if (mknod(file_path, 0644, 0) < 0) {
+    err(1, "mknod(%s)", file_path);
+  }
+
+  // After rewinddir(), re-reading the directory stream should yield the new
+  // file.
+  rewinddir(dir);
+  size_t found_file = 0;
+  while (1) {
+    errno = 0;
+    struct dirent* d = readdir(dir);
+    if (!d) {
+      if (errno != 0) {
+        err(1, "readdir");
+      }
+      break;
+    }
+    if (strcmp(d->d_name, kFileBasename) == 0) {
+      found_file++;
+    } else if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0) {
+      warnx("unexpected file %s in new directory", d->d_name);
+      failed = 1;
+    }
+  }
+  if (found_file != 1) {
+    warnx("readdir returned file %s %zu times, wanted 1", kFileBasename,
+          found_file);
+    failed = 1;
+  }
+
+  return failed;
+}
diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go
index ebefeacf2..c6694c278 100644
--- a/runsc/fsgofer/fsgofer.go
+++ b/runsc/fsgofer/fsgofer.go
@@ -979,9 +979,12 @@ func (l *localFile) Readdir(offset uint64, count uint32) ([]p9.Dirent, error) {
 
 	skip := uint64(0)
 
-	// Check if the file is at the correct position already. If not, seek to the
-	// beginning and read the entire directory again.
-	if l.lastDirentOffset != offset {
+	// Check if the file is at the correct position already. If not, seek to
+	// the beginning and read the entire directory again. We always seek if
+	// offset is 0, since this is side-effectual (equivalent to rewinddir(3),
+	// which causes the directory stream to resynchronize with the directory's
+	// current contents).
+	if l.lastDirentOffset != offset || offset == 0 {
 		if _, err := syscall.Seek(l.file.FD(), 0, 0); err != nil {
 			return nil, extractErrno(err)
 		}
diff --git a/test/e2e/integration_test.go b/test/e2e/integration_test.go
index 5465dee9b..6fe6d304f 100644
--- a/test/e2e/integration_test.go
+++ b/test/e2e/integration_test.go
@@ -434,7 +434,33 @@ func TestHostOverlayfsCopyUp(t *testing.T) {
 	if got, err := d.Run(ctx, dockerutil.RunOpts{
 		Image:   "basic/hostoverlaytest",
 		WorkDir: "/root",
-	}, "./test"); err != nil {
+	}, "./test_copy_up"); err != nil {
+		t.Fatalf("docker run failed: %v", err)
+	} else if got != "" {
+		t.Errorf("test failed:\n%s", got)
+	}
+}
+
+// TestHostOverlayfsRewindDir tests that rewinddir() "causes the directory
+// stream to refer to the current state of the corresponding directory, as a
+// call to opendir() would have done" as required by POSIX, when the directory
+// in question is host overlayfs.
+//
+// This test specifically targets host overlayfs because, per POSIX, "if a file
+// is removed from or added to the directory after the most recent call to
+// opendir() or rewinddir(), whether a subsequent call to readdir() returns an
+// entry for that file is unspecified"; the host filesystems used by other
+// automated tests yield newly-added files from readdir() even if the fsgofer
+// does not explicitly rewinddir(), but overlayfs does not.
+func TestHostOverlayfsRewindDir(t *testing.T) {
+	ctx := context.Background()
+	d := dockerutil.MakeContainer(ctx, t)
+	defer d.CleanUp(ctx)
+
+	if got, err := d.Run(ctx, dockerutil.RunOpts{
+		Image:   "basic/hostoverlaytest",
+		WorkDir: "/root",
+	}, "./test_rewinddir"); err != nil {
 		t.Fatalf("docker run failed: %v", err)
 	} else if got != "" {
 		t.Errorf("test failed:\n%s", got)