Add fh support for revise attr and fstat(2) test

According to Linux 4.4's FUSE behavior, the flags and fh attributes in
FUSE_GETATTR are only used in read, write, and lseek. fstat(2) doesn't
use them either. Add tests to ensure the requests sent from FUSE module
are consistent with Linux's.

Updates #3655

7 files changed, 133 insertions, 28 deletions

diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go
index e49a92fb2..f0bef1e8e 100644
--- a/pkg/abi/linux/fuse.go
+++ b/pkg/abi/linux/fuse.go
@@ -227,6 +227,11 @@ type FUSEInitOut struct {
 	_ [8]uint32
 }
 
+// FUSE_GETATTR_FH is currently the only flag of FUSEGetAttrIn.GetAttrFlags.
+// If it is set, the file handle (FUSEGetAttrIn.Fh) is used to indicate the
+// object instead of the node id attribute in the request header.
+const FUSE_GETATTR_FH = (1 << 0)
+
 // FUSEGetAttrIn is the request sent by the kernel to the daemon,
 // to get the attribute of a inode.
 //
diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go
index 8cf13dcb6..821048d87 100644
--- a/pkg/sentry/fsimpl/fuse/fusefs.go
+++ b/pkg/sentry/fsimpl/fuse/fusefs.go
@@ -609,9 +609,9 @@ func statFromFUSEAttr(attr linux.FUSEAttr, mask, devMinor uint32) linux.Statx {
 }
 
 // getAttr gets the attribute of this inode by issuing a FUSE_GETATTR request
-// or read from local cache.
-// It updates the corresponding attributes if necessary.
-func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOptions) (linux.FUSEAttr, error) {
+// or read from local cache. It updates the corresponding attributes if
+// necessary.
+func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOptions, flags uint32, fh uint64) (linux.FUSEAttr, error) {
 	attributeVersion := atomic.LoadUint64(&i.fs.conn.attributeVersion)
 
 	// TODO(gvisor.dev/issue/3679): send the request only if
@@ -631,11 +631,10 @@ func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOp
 
 	creds := auth.CredentialsFromContext(ctx)
 
-	var in linux.FUSEGetAttrIn
-	// We don't set any attribute in the request, because in VFS2 fstat(2) will
-	// finally be translated into vfs.FilesystemImpl.StatAt() (see
-	// pkg/sentry/syscalls/linux/vfs2/stat.go), resulting in the same flow
-	// as stat(2). Thus GetAttrFlags and Fh variable will never be used in VFS2.
+	in := linux.FUSEGetAttrIn{
+		GetAttrFlags: flags,
+		Fh:           fh,
+	}
 	req, err := i.fs.conn.NewRequest(creds, uint32(task.ThreadID()), i.NodeID, linux.FUSE_GETATTR, &in)
 	if err != nil {
 		return linux.FUSEAttr{}, err
@@ -676,17 +675,17 @@ func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOp
 // reviseAttr attempts to update the attributes for internal purposes
 // by calling getAttr with a pre-specified mask.
 // Used by read, write, lseek.
-func (i *inode) reviseAttr(ctx context.Context) error {
+func (i *inode) reviseAttr(ctx context.Context, flags uint32, fh uint64) error {
 	// Never need atime for internal purposes.
 	_, err := i.getAttr(ctx, i.fs.VFSFilesystem(), vfs.StatOptions{
 		Mask: linux.STATX_BASIC_STATS &^ linux.STATX_ATIME,
-	})
+	}, flags, fh)
 	return err
 }
 
 // Stat implements kernfs.Inode.Stat.
 func (i *inode) Stat(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOptions) (linux.Statx, error) {
-	attr, err := i.getAttr(ctx, fs, opts)
+	attr, err := i.getAttr(ctx, fs, opts, 0, 0)
 	if err != nil {
 		return linux.Statx{}, err
 	}
diff --git a/pkg/sentry/fsimpl/fuse/regular_file.go b/pkg/sentry/fsimpl/fuse/regular_file.go
index 193e77392..5bdd096c3 100644
--- a/pkg/sentry/fsimpl/fuse/regular_file.go
+++ b/pkg/sentry/fsimpl/fuse/regular_file.go
@@ -65,7 +65,7 @@ func (fd *regularFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs
 
 	// Reading beyond EOF, update file size if outdated.
 	if uint64(offset+size) > atomic.LoadUint64(&inode.size) {
-		if err := inode.reviseAttr(ctx); err != nil {
+		if err := inode.reviseAttr(ctx, linux.FUSE_GETATTR_FH, fd.Fh); err != nil {
 			return 0, err
 		}
 		// If the offset after update is still too large, return error.
diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD
index eb090cbfd..7a3e52fad 100644
--- a/test/fuse/linux/BUILD
+++ b/test/fuse/linux/BUILD
@@ -11,7 +11,9 @@ cc_binary(
     srcs = ["stat_test.cc"],
     deps = [
         gtest,
-        ":fuse_base",
+        ":fuse_fd_util",
+        "//test/util:cleanup",
+        "//test/util:fs_util",
         "//test/util:fuse_util",
         "//test/util:test_main",
         "//test/util:test_util",
diff --git a/test/fuse/linux/stat_test.cc b/test/fuse/linux/stat_test.cc
index 717fd1fac..6f032cac1 100644
--- a/test/fuse/linux/stat_test.cc
+++ b/test/fuse/linux/stat_test.cc
@@ -18,12 +18,15 @@
 #include <sys/stat.h>
 #include <sys/statfs.h>
 #include <sys/types.h>
+#include <sys/uio.h>
 #include <unistd.h>
 
 #include <vector>
 
 #include "gtest/gtest.h"
-#include "test/fuse/linux/fuse_base.h"
+#include "test/fuse/linux/fuse_fd_util.h"
+#include "test/util/cleanup.h"
+#include "test/util/fs_util.h"
 #include "test/util/fuse_util.h"
 #include "test/util/test_util.h"
 
@@ -32,19 +35,30 @@ namespace testing {
 
 namespace {
 
-class StatTest : public FuseTest {
+class StatTest : public FuseFdTest {
  public:
+  void SetUp() override {
+    FuseFdTest::SetUp();
+    test_file_path_ = JoinPath(mount_point_.path(), test_file_);
+  }
+
+ protected:
   bool StatsAreEqual(struct stat expected, struct stat actual) {
-    // device number will be dynamically allocated by kernel, we cannot know
-    // in advance
+    // Device number will be dynamically allocated by kernel, we cannot know in
+    // advance.
     actual.st_dev = expected.st_dev;
     return memcmp(&expected, &actual, sizeof(struct stat)) == 0;
   }
+
+  const std::string test_file_ = "testfile";
+  const mode_t expected_mode = S_IFREG | S_IRUSR | S_IWUSR;
+  const uint64_t fh = 23;
+
+  std::string test_file_path_;
 };
 
 TEST_F(StatTest, StatNormal) {
   // Set up fixture.
-  mode_t expected_mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
   struct fuse_attr attr = DefaultFuseAttr(expected_mode, 1);
   struct fuse_out_header out_header = {
       .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out),
@@ -55,7 +69,7 @@ TEST_F(StatTest, StatNormal) {
   auto iov_out = FuseGenerateIovecs(out_header, out_payload);
   SetServerResponse(FUSE_GETATTR, iov_out);
 
-  // Do integration test.
+  // Make syscall.
   struct stat stat_buf;
   EXPECT_THAT(stat(mount_point_.path().c_str(), &stat_buf), SyscallSucceeds());
 
@@ -99,7 +113,7 @@ TEST_F(StatTest, StatNotFound) {
   auto iov_out = FuseGenerateIovecs(out_header);
   SetServerResponse(FUSE_GETATTR, iov_out);
 
-  // Do integration test.
+  // Make syscall.
   struct stat stat_buf;
   EXPECT_THAT(stat(mount_point_.path().c_str(), &stat_buf),
               SyscallFailsWithErrno(ENOENT));
@@ -115,6 +129,90 @@ TEST_F(StatTest, StatNotFound) {
   EXPECT_EQ(in_payload.fh, 0);
 }
 
+TEST_F(StatTest, FstatNormal) {
+  // Set up fixture.
+  SetServerInodeLookup(test_file_);
+  auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenPath(test_file_path_, O_RDONLY, fh));
+  auto close_fd = CloseFD(fd);
+
+  struct fuse_attr attr = DefaultFuseAttr(expected_mode, 2);
+  struct fuse_out_header out_header = {
+      .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out),
+  };
+  struct fuse_attr_out out_payload = {
+      .attr = attr,
+  };
+  auto iov_out = FuseGenerateIovecs(out_header, out_payload);
+  SetServerResponse(FUSE_GETATTR, iov_out);
+
+  // Make syscall.
+  struct stat stat_buf;
+  EXPECT_THAT(fstat(fd.get(), &stat_buf), SyscallSucceeds());
+
+  // Check filesystem operation result.
+  struct stat expected_stat = {
+      .st_ino = attr.ino,
+      .st_nlink = attr.nlink,
+      .st_mode = expected_mode,
+      .st_uid = attr.uid,
+      .st_gid = attr.gid,
+      .st_rdev = attr.rdev,
+      .st_size = static_cast<off_t>(attr.size),
+      .st_blksize = attr.blksize,
+      .st_blocks = static_cast<blkcnt_t>(attr.blocks),
+      .st_atim = (struct timespec){.tv_sec = static_cast<int>(attr.atime),
+                                   .tv_nsec = attr.atimensec},
+      .st_mtim = (struct timespec){.tv_sec = static_cast<int>(attr.mtime),
+                                   .tv_nsec = attr.mtimensec},
+      .st_ctim = (struct timespec){.tv_sec = static_cast<int>(attr.ctime),
+                                   .tv_nsec = attr.ctimensec},
+  };
+  EXPECT_TRUE(StatsAreEqual(stat_buf, expected_stat));
+
+  // Check FUSE request.
+  struct fuse_in_header in_header;
+  struct fuse_getattr_in in_payload;
+  auto iov_in = FuseGenerateIovecs(in_header, in_payload);
+
+  GetServerActualRequest(iov_in);
+  EXPECT_EQ(in_header.opcode, FUSE_GETATTR);
+  EXPECT_EQ(in_payload.getattr_flags, 0);
+  EXPECT_EQ(in_payload.fh, 0);
+}
+
+TEST_F(StatTest, StatByFileHandle) {
+  // Set up fixture.
+  SetServerInodeLookup(test_file_, expected_mode, 0);
+  auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenPath(test_file_path_, O_RDONLY, fh));
+  auto close_fd = CloseFD(fd);
+
+  struct fuse_attr attr = DefaultFuseAttr(expected_mode, 2, 0);
+  struct fuse_out_header out_header = {
+      .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out),
+  };
+  struct fuse_attr_out out_payload = {
+      .attr = attr,
+  };
+  auto iov_out = FuseGenerateIovecs(out_header, out_payload);
+  SetServerResponse(FUSE_GETATTR, iov_out);
+
+  // Make syscall.
+  std::vector<char> buf(1);
+  // Since this is an empty file, it won't issue FUSE_READ. But a FUSE_GETATTR
+  // will be issued before read completes.
+  EXPECT_THAT(read(fd.get(), buf.data(), buf.size()), SyscallSucceeds());
+
+  // Check FUSE request.
+  struct fuse_in_header in_header;
+  struct fuse_getattr_in in_payload;
+  auto iov_in = FuseGenerateIovecs(in_header, in_payload);
+
+  GetServerActualRequest(iov_in);
+  EXPECT_EQ(in_header.opcode, FUSE_GETATTR);
+  EXPECT_EQ(in_payload.getattr_flags, FUSE_GETATTR_FH);
+  EXPECT_EQ(in_payload.fh, fh);
+}
+
 }  // namespace
 
 }  // namespace testing
diff --git a/test/util/fuse_util.cc b/test/util/fuse_util.cc
index 4db053335..595d0cebf 100644
--- a/test/util/fuse_util.cc
+++ b/test/util/fuse_util.cc
@@ -22,13 +22,13 @@
 namespace gvisor {
 namespace testing {
 
-// Create a default FuseAttr struct with specified mode and inode.
-fuse_attr DefaultFuseAttr(mode_t mode, uint64_t inode) {
+// Create a default FuseAttr struct with specified mode, inode, and size.
+fuse_attr DefaultFuseAttr(mode_t mode, uint64_t inode, uint64_t size) {
   const int time_sec = 1595436289;
   const int time_nsec = 134150844;
   return (struct fuse_attr){
       .ino = inode,
-      .size = 512,
+      .size = size,
       .blocks = 4,
       .atime = time_sec,
       .mtime = time_sec,
@@ -45,8 +45,8 @@ fuse_attr DefaultFuseAttr(mode_t mode, uint64_t inode) {
   };
 }
 
-// Create response body with specified mode and nodeID.
-fuse_entry_out DefaultEntryOut(mode_t mode, uint64_t node_id) {
+// Create response body with specified mode, nodeID, and size.
+fuse_entry_out DefaultEntryOut(mode_t mode, uint64_t node_id, uint64_t size) {
   struct fuse_entry_out default_entry_out = {
       .nodeid = node_id,
       .generation = 0,
@@ -54,7 +54,7 @@ fuse_entry_out DefaultEntryOut(mode_t mode, uint64_t node_id) {
       .attr_valid = 0,
       .entry_valid_nsec = 0,
       .attr_valid_nsec = 0,
-      .attr = DefaultFuseAttr(mode, node_id),
+      .attr = DefaultFuseAttr(mode, node_id, size),
   };
   return default_entry_out;
 };
diff --git a/test/util/fuse_util.h b/test/util/fuse_util.h
index 6b5a8ce1f..544fe1b38 100644
--- a/test/util/fuse_util.h
+++ b/test/util/fuse_util.h
@@ -64,10 +64,11 @@ std::vector<struct iovec> FuseGenerateIovecs(T &first, Types &...args) {
 }
 
 // Create a fuse_attr filled with the specified mode and inode.
-fuse_attr DefaultFuseAttr(mode_t mode, uint64_t inode);
+fuse_attr DefaultFuseAttr(mode_t mode, uint64_t inode, uint64_t size = 512);
 
 // Return a fuse_entry_out FUSE server response body.
-fuse_entry_out DefaultEntryOut(mode_t mode, uint64_t nodeId);
+fuse_entry_out DefaultEntryOut(mode_t mode, uint64_t node_id,
+                               uint64_t size = 512);
 
 }  // namespace testing
 }  // namespace gvisor