fuse: Implement IterDirents for directory file description

Fixes #3255.

This change adds support for IterDirents. You can now use `ls` in
the FUSE sandbox.

Co-authored-by: Craig Chi <craigchi@google.com>

pkg/abi/linux/fuse.go

@@ -14,7 +14,10 @@
 
 package linux
 
-import "gvisor.dev/gvisor/tools/go_marshal/marshal"
+import (
+	"gvisor.dev/gvisor/tools/go_marshal/marshal"
+	"gvisor.dev/gvisor/tools/go_marshal/primitive"
+)
 
 // +marshal
 type FUSEOpcode uint32
@@ -186,9 +189,9 @@ const (
 
 // Constants relevant to FUSE operations.
 const (
-	FUSE_NAME_MAX     = 1024
-	FUSE_PAGE_SIZE    = 4096
-	FUSE_DIRENT_ALIGN = 8
+	FUSE_NAME_MAX       = 1024
+	FUSE_PAGE_SIZE      = 4096
+	FUSE_DIRENT_ALIGN   = 8
 )
 
 // FUSEInitIn is the request sent by the kernel to the daemon,
@@ -595,3 +598,123 @@ func (r *FUSERmDirIn) SizeBytes() int {
 func (r *FUSERmDirIn) UnmarshalUnsafe(src []byte) {
 	r.Name = string(src)
 }
+
+// FUSEDirents is a list of Dirents received from the FUSE daemon server.
+// It is used for FUSE_READDIR.
+//
+// Dynamically-sized objects cannot be marshalled.
+type FUSEDirents struct {
+	marshal.StubMarshallable
+
+	Dirents []*FUSEDirent
+}
+
+// FUSEDirent is a Dirent received from the FUSE daemon server.
+// It is used for FUSE_READDIR.
+//
+// Dynamically-sized objects cannot be marshalled.
+type FUSEDirent struct {
+	marshal.StubMarshallable
+
+	// Meta contains all the static fields of FUSEDirent.
+	Meta FUSEDirentMeta
+
+	// Name is the filename of the dirent.
+	Name string
+}
+
+// FUSEDirentMeta contains all the static fields of FUSEDirent.
+// It is used for FUSE_READDIR.
+//
+// +marshal
+type FUSEDirentMeta struct {
+	// Inode of the dirent.
+	Ino uint64
+
+	// Offset of the dirent.
+	Off uint64
+
+	// NameLen is the length of the dirent name.
+	NameLen uint32
+
+	// Type of the dirent.
+	Type uint32
+}
+
+// MarshalUnsafe serializes FUSEDirents to the dst buffer.
+func (r *FUSEDirents) MarshalUnsafe(dst []byte) {
+	for _, dirent := range r.Dirents {
+		dirent.MarshalUnsafe(dst)
+		dst = dst[dirent.SizeBytes():]
+	}
+}
+
+// SizeBytes is the size of the memory representation of FUSEDirents.
+func (r *FUSEDirents) SizeBytes() int {
+	var sizeBytes int
+	for _, dirent := range r.Dirents {
+		sizeBytes += dirent.SizeBytes()
+	}
+
+	return sizeBytes
+}
+
+// UnmarshalUnsafe deserializes FUSEDirents from the src buffer.
+func (r *FUSEDirents) UnmarshalUnsafe(src []byte) {
+	for {
+		if len(src) <= (*FUSEDirentMeta)(nil).SizeBytes() {
+			break
+		}
+
+		// Its unclear how many dirents there are in src. Each dirent is dynamically
+		// sized and so we can't make assumptions about how many dirents we can allocate.
+		if r.Dirents == nil {
+			r.Dirents = make([]*FUSEDirent, 0)
+		}
+
+		// We have to allocate a struct for each dirent - there must be a better way
+		// to do this. Linux allocates 1 page to store all the dirents and then
+		// simply reads them from the page.
+		var dirent FUSEDirent
+		dirent.UnmarshalUnsafe(src)
+		r.Dirents = append(r.Dirents, &dirent)
+
+		src = src[dirent.SizeBytes():]
+	}
+}
+
+// MarshalUnsafe serializes FUSEDirent to the dst buffer.
+func (r *FUSEDirent) MarshalUnsafe(dst []byte) {
+	r.Meta.MarshalUnsafe(dst)
+	dst = dst[r.Meta.SizeBytes():]
+
+	name := primitive.ByteSlice(r.Name)
+	name.MarshalUnsafe(dst)
+}
+
+// SizeBytes is the size of the memory representation of FUSEDirent.
+func (r *FUSEDirent) SizeBytes() int {
+	dataSize := r.Meta.SizeBytes() + len(r.Name)
+
+	// Each Dirent must be padded such that its size is a multiple
+	// of FUSE_DIRENT_ALIGN. Similar to the fuse dirent alignment
+	// in linux/fuse.h.
+	return (dataSize + (FUSE_DIRENT_ALIGN - 1)) & ^(FUSE_DIRENT_ALIGN - 1)
+}
+
+// UnmarshalUnsafe deserializes FUSEDirent from the src buffer.
+func (r *FUSEDirent) UnmarshalUnsafe(src []byte) {
+	r.Meta.UnmarshalUnsafe(src)
+	src = src[r.Meta.SizeBytes():]
+
+	if r.Meta.NameLen > FUSE_NAME_MAX {
+		// The name is too long and therefore invalid. We don't
+		// need to unmarshal the name since it'll be thrown away.
+		return
+	}
+
+	buf := make([]byte, r.Meta.NameLen)
+	name := primitive.ByteSlice(buf)
+	name.UnmarshalUnsafe(src[:r.Meta.NameLen])
+	r.Name = string(name)
+}

pkg/sentry/fsimpl/fuse/connection.go

@@ -21,6 +21,8 @@ import (
 	"sync/atomic"
 	"syscall"
 
+	"gvisor.dev/gvisor/tools/go_marshal/marshal"
+
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/log"
@@ -352,6 +354,12 @@ func (r *Response) UnmarshalPayload(m marshal.Marshallable) error {
 		return fmt.Errorf("payload too small. Minimum data lenth required: %d,  but got data length %d", wantDataLen, haveDataLen)
 	}
 
+	// The response data is empty unless there is some payload. And so, doesn't
+	// need to be unmarshalled.
+	if r.data == nil {
+		return nil
+	}
+
 	m.UnmarshalUnsafe(r.data[hdrLen:])
 	return nil
 }

pkg/sentry/fsimpl/fuse/directory.go

@@ -15,7 +15,12 @@
 package fuse
 
 import (
+	"sync/atomic"
+
+	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/context"
+	"gvisor.dev/gvisor/pkg/sentry/kernel"
+	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
 	"gvisor.dev/gvisor/pkg/syserror"
 	"gvisor.dev/gvisor/pkg/usermem"
@@ -49,3 +54,52 @@ func (directoryFD) PWrite(ctx context.Context, src usermem.IOSequence, offset in
 func (directoryFD) Write(ctx context.Context, src usermem.IOSequence, opts vfs.WriteOptions) (int64, error) {
 	return 0, syserror.EISDIR
 }
+
+// IterDirents implements FileDescriptionImpl.IterDirents.
+func (dir *directoryFD) IterDirents(ctx context.Context, callback vfs.IterDirentsCallback) error {
+	fusefs := dir.inode().fs
+	task, creds := kernel.TaskFromContext(ctx), auth.CredentialsFromContext(ctx)
+
+	in := linux.FUSEReadIn{
+		Fh:     dir.Fh,
+		Offset: uint64(atomic.LoadInt64(&dir.off)),
+		Size:   linux.FUSE_PAGE_SIZE,
+		Flags:  dir.statusFlags(),
+	}
+
+	/// TODO(gVisor.dev/issue/3404): Support FUSE_READDIRPLUS.
+	req, err := fusefs.conn.NewRequest(creds, uint32(task.ThreadID()), dir.inode().NodeID, linux.FUSE_READDIR, &in)
+	if err != nil {
+		return err
+	}
+
+	res, err := fusefs.conn.Call(task, req)
+	if err != nil {
+		return err
+	}
+	if err := res.Error(); err != nil {
+		return err
+	}
+
+	var out linux.FUSEDirents
+	if err := res.UnmarshalPayload(&out); err != nil {
+		return err
+	}
+
+	for _, fuseDirent := range out.Dirents {
+		nextOff := int64(fuseDirent.Meta.Off) + 1
+		dirent := vfs.Dirent{
+			Name:    fuseDirent.Name,
+			Type:    uint8(fuseDirent.Meta.Type),
+			Ino:     fuseDirent.Meta.Ino,
+			NextOff: nextOff,
+		}
+
+		if err := callback.Handle(dirent); err != nil {
+			return err
+		}
+		atomic.StoreInt64(&dir.off, nextOff)
+	}
+
+	return nil
+}

pkg/sentry/fsimpl/fuse/file.go

@@ -42,6 +42,9 @@ type fileDescription struct {
 
 	// OpenFlag is the flag returned by open.
 	OpenFlag uint32
+
+	// off is the file offset.
+	off int64
 }
 
 func (fd *fileDescription) dentry() *kernfs.Dentry {
@@ -119,5 +122,6 @@ func (fd *fileDescription) Stat(ctx context.Context, opts vfs.StatOptions) (linu
 
 // SetStat implements FileDescriptionImpl.SetStat.
 func (fd *fileDescription) SetStat(ctx context.Context, opts vfs.SetStatOptions) error {
-	return nil
+	creds := auth.CredentialsFromContext(ctx)
+	return fd.inode().SetStat(ctx, fd.inode().fs.VFSFilesystem(), creds, opts)
 }

pkg/sentry/fsimpl/kernfs/kernfs.go

@@ -425,7 +425,7 @@ type inodeDynamicLookup interface {
 	Valid(ctx context.Context) bool
 
 	// IterDirents is used to iterate over dynamically created entries. It invokes
-	// cb on each entry in the directory represented by the FileDescription.
+	// cb on each entry in the directory represented by the Inode.
 	// 'offset' is the offset for the entire IterDirents call, which may include
 	// results from the caller (e.g. "." and ".."). 'relOffset' is the offset
 	// inside the entries returned by this IterDirents invocation. In other words,

pkg/sentry/vfs/file_description_impl_util.go

@@ -107,7 +107,7 @@ func (FileDescriptionDefaultImpl) Write(ctx context.Context, src usermem.IOSeque
 // file_operations::iterate == file_operations::iterate_shared == NULL in
 // Linux.
 func (FileDescriptionDefaultImpl) IterDirents(ctx context.Context, cb IterDirentsCallback) error {
-	return syserror.ENOTDIR
+	return syserror.ENOSYS
 }
 
 // Seek implements FileDescriptionImpl.Seek analogously to

test/fuse/BUILD

@@ -43,7 +43,11 @@ syscall_test(
 )
 
 syscall_test(
-    test = "//test/fuse/linux:rmdir_test",
-    vfs2 = "True",
     fuse = "True",
+    test = "//test/fuse/linux:rmdir_test",
+)
+
+syscall_test(
+    fuse = "True",
+    test = "//test/fuse/linux:readdir_test",
 )

test/fuse/linux/BUILD

@@ -112,6 +112,20 @@ cc_binary(
     ],
 )
 
+cc_binary(
+    name = "readdir_test",
+    testonly = 1,
+    srcs = ["readdir_test.cc"],
+    deps = [
+        gtest,
+        ":fuse_base",
+        "//test/util:fs_util",
+        "//test/util:fuse_util",
+        "//test/util:test_main",
+        "//test/util:test_util",
+    ],
+)
+
 cc_library(
     name = "fuse_base",
     testonly = 1,

test/fuse/linux/readdir_test.cc

@@ -0,0 +1,188 @@
+// Copyright 2020 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/fuse.h>
+#include <sys/stat.h>
+#include <sys/statfs.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <string>
+
+#include "gtest/gtest.h"
+#include "test/fuse/linux/fuse_base.h"
+#include "test/util/fuse_util.h"
+#include "test/util/test_util.h"
+
+#define FUSE_NAME_OFFSET offsetof(struct fuse_dirent, name)
+#define FUSE_DIRENT_ALIGN(x) \
+  (((x) + sizeof(uint64_t) - 1) & ~(sizeof(uint64_t) - 1))
+#define FUSE_DIRENT_SIZE(d) FUSE_DIRENT_ALIGN(FUSE_NAME_OFFSET + (d)->namelen)
+
+namespace gvisor {
+namespace testing {
+
+namespace {
+
+class ReaddirTest : public FuseTest {
+ public:
+  void fill_fuse_dirent(char *buf, const char *name) {
+    size_t namelen = strlen(name);
+    size_t entlen = FUSE_NAME_OFFSET + namelen;
+    size_t entlen_padded = FUSE_DIRENT_ALIGN(entlen);
+    struct fuse_dirent *dirent;
+
+    dirent = reinterpret_cast<struct fuse_dirent *>(buf);
+    dirent->namelen = namelen;
+    memcpy(dirent->name, name, namelen);
+    memset(dirent->name + namelen, 0, entlen_padded - entlen);
+  }
+
+ protected:
+  const std::string test_dir_name_ = "test_dir";
+};
+
+TEST_F(ReaddirTest, SingleEntry) {
+  const std::string test_dir_path =
+      JoinPath(mount_point_.path().c_str(), test_dir_name_);
+
+  // We need to make sure the test dir is a directory that can be found.
+  mode_t expected_mode =
+      S_IFDIR | S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
+  struct fuse_attr dir_attr = {
+      .ino = 1,
+      .size = 512,
+      .blocks = 4,
+      .mode = expected_mode,
+      .blksize = 4096,
+  };
+  struct fuse_out_header stat_header = {
+      .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out),
+  };
+
+  struct fuse_attr_out stat_payload = {
+      .attr_valid_nsec = 2,
+      .attr = dir_attr,
+  };
+
+  // We need to make sure the test dir is a directory that can be found.
+  struct fuse_out_header lookup_header = {
+      .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out),
+  };
+  struct fuse_entry_out lookup_payload = {
+      .nodeid = 1,
+      .entry_valid = true,
+      .attr_valid = true,
+      .attr = dir_attr,
+  };
+
+  struct fuse_out_header open_header = {
+      .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_open_out),
+  };
+  struct fuse_open_out open_payload = {
+      .fh = 1,
+  };
+  auto iov_out = FuseGenerateIovecs(lookup_header, lookup_payload);
+  SetServerResponse(FUSE_LOOKUP, iov_out);
+
+  iov_out = FuseGenerateIovecs(open_header, open_payload);
+  SetServerResponse(FUSE_OPENDIR, iov_out);
+
+  iov_out = FuseGenerateIovecs(stat_header, stat_payload);
+  SetServerResponse(FUSE_GETATTR, iov_out);
+
+  DIR *dir = opendir(test_dir_path.c_str());
+
+  // The opendir command makes three syscalls. Lookup the dir file, stat it and
+  // open.
+  // We don't need to inspect those headers in this test.
+  SkipServerActualRequest();  // LOOKUP.
+  SkipServerActualRequest();  // GETATTR.
+  SkipServerActualRequest();  // OPENDIR.
+
+  // Readdir test code.
+  std::string dot = ".";
+  std::string dot_dot = "..";
+  std::string test_file = "testFile";
+
+  // Figure out how many dirents to send over and allocate them appropriately.
+  // Each dirent has a dynamic name and a static metadata part. The dirent size
+  // is aligned to being a multiple of 8.
+  size_t dot_file_dirent_size =
+      FUSE_DIRENT_ALIGN(dot.length() + FUSE_NAME_OFFSET);
+  size_t dot_dot_file_dirent_size =
+      FUSE_DIRENT_ALIGN(dot_dot.length() + FUSE_NAME_OFFSET);
+  size_t test_file_dirent_size =
+      FUSE_DIRENT_ALIGN(test_file.length() + FUSE_NAME_OFFSET);
+
+  // Create an appropriately sized payload.
+  size_t readdir_payload_size =
+      test_file_dirent_size + dot_file_dirent_size + dot_dot_file_dirent_size;
+  char readdir_payload[readdir_payload_size];
+
+  fill_fuse_dirent(readdir_payload, dot.c_str());
+  fill_fuse_dirent(readdir_payload + dot_file_dirent_size, dot_dot.c_str());
+  fill_fuse_dirent(
+      readdir_payload + dot_file_dirent_size + dot_dot_file_dirent_size,
+      test_file.c_str());
+
+  std::vector<char> readdir_payload_vec(readdir_payload,
+                                        readdir_payload + readdir_payload_size);
+  struct fuse_out_header readdir_header = {
+      .len = uint32_t(sizeof(struct fuse_out_header) + sizeof(readdir_payload)),
+  };
+  struct fuse_out_header readdir_header_break = {
+      .len = uint32_t(sizeof(struct fuse_out_header)),
+  };
+
+  iov_out = FuseGenerateIovecs(readdir_header, readdir_payload_vec);
+  SetServerResponse(FUSE_READDIR, iov_out);
+
+  iov_out = FuseGenerateIovecs(readdir_header_break);
+  SetServerResponse(FUSE_READDIR, iov_out);
+
+  struct dirent *entry;
+  entry = readdir(dir);
+  EXPECT_EQ(std::string(entry->d_name), dot);
+
+  entry = readdir(dir);
+  EXPECT_EQ(std::string(entry->d_name), dot_dot);
+
+  entry = readdir(dir);
+  EXPECT_EQ(std::string(entry->d_name), test_file);
+
+  entry = readdir(dir);
+  EXPECT_TRUE((entry == NULL));
+
+  SkipServerActualRequest();  // READDIR.
+  SkipServerActualRequest();  // READDIR with no data.
+
+  // Clean up.
+  closedir(dir);
+
+  struct fuse_in_header in_header;
+  struct fuse_release_in in_payload;
+
+  auto iov_in = FuseGenerateIovecs(in_header, in_payload);
+  GetServerActualRequest(iov_in);
+  EXPECT_EQ(in_header.len, sizeof(in_header) + sizeof(in_payload));
+  EXPECT_EQ(in_header.opcode, FUSE_RELEASEDIR);
+}
+
+}  // namespace
+
+}  // namespace testing
+}  // namespace gvisor