Modify FUSE inodes so they're not always assumed to be valid.

PiperOrigin-RevId: 621954030

pkg/sentry/fsimpl/fuse/fusefs.go

@@ -301,13 +301,17 @@ func (fs *filesystem) newRoot(ctx context.Context, creds *auth.Credentials, mode
 	return &d
 }
 
-func (fs *filesystem) newInode(ctx context.Context, nodeID uint64, attr linux.FUSEAttr) kernfs.Inode {
+func (fs *filesystem) newInode(ctx context.Context, nodeID uint64, out linux.FUSEEntryOut) kernfs.Inode {
+	attr := out.Attr
 	i := &inode{fs: fs, nodeID: nodeID}
-	creds := auth.Credentials{EffectiveKGID: auth.KGID(attr.UID), EffectiveKUID: auth.KUID(attr.UID)}
+	i.updateEntryTime(int64(out.EntryValid), int64(out.EntryValidNSec))
 	i.attrMu.Lock()
+	defer i.attrMu.Unlock()
+
+	creds := auth.Credentials{EffectiveKGID: auth.KGID(attr.UID), EffectiveKUID: auth.KUID(attr.UID)}
 	i.init(&creds, linux.UNNAMED_MAJOR, fs.devMinor, nodeID, linux.FileMode(attr.Mode), attr.Nlink)
-	i.size.Store(attr.Size)
-	i.attrMu.Unlock()
+	i.updateAttrs(attr, int64(out.AttrValid), int64(out.AttrValidNSec))
+
 	i.OrderedChildren.Init(kernfs.OrderedChildrenOptions{})
 	i.InitRefs()
 	return i

pkg/sentry/fsimpl/fuse/inode.go

@@ -17,9 +17,10 @@ package fuse
 import (
 	"fmt"
 	"sync"
-	"time"
+	gotime "time"
 
 	"gvisor.dev/gvisor/pkg/abi/linux"
+
 	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
@@ -29,6 +30,7 @@ import (
 	"gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs"
 	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
+	"gvisor.dev/gvisor/pkg/sentry/kernel/time"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
 )
 
@@ -44,7 +46,6 @@ type fileHandle struct {
 // +stateify savable
 type inode struct {
 	inodeRefs
-	kernfs.InodeAlwaysValid
 	kernfs.InodeNotAnonymous
 	kernfs.InodeNotSymlink
 	kernfs.InodeWatches
@@ -58,11 +59,14 @@ type inode struct {
 	// and the sentry. Immutable.
 	nodeID uint64
 
+	// entryTime is the time at which the entry becomes invalid.
+	entryTime time.Time
+
 	// attrVersion is the version of the last attribute change.
 	attrVersion atomicbitops.Uint64
 
-	// attrTime is the time until the attributes are valid.
-	attrTime uint64
+	// attrTime is the time at which the attributes become invalid.
+	attrTime time.Time
 
 	// link is result of following a symbolic link.
 	link string
@@ -189,6 +193,11 @@ func (i *inode) init(creds *auth.Credentials, devMajor, devMinor uint32, nodeid
 	i.ctime.Store(now)
 }
 
+func (i *inode) updateEntryTime(entrySec, entryNSec int64) {
+	entryTime := time.FromTimespec(linux.Timespec{Sec: entrySec, Nsec: entryNSec})
+	i.entryTime = i.fs.clock.Now().AddTime(entryTime)
+}
+
 // CheckPermissions implements kernfs.Inode.CheckPermissions.
 func (i *inode) CheckPermissions(ctx context.Context, creds *auth.Credentials, ats vfs.AccessTypes) error {
 	// Since FUSE operations are ultimately backed by a userspace process (the
@@ -219,7 +228,7 @@ func (i *inode) CheckPermissions(ctx context.Context, creds *auth.Credentials, a
 	refreshed := false
 	opts := vfs.StatOptions{Mask: linux.STATX_MODE | linux.STATX_UID | linux.STATX_GID}
 	if i.fs.opts.defaultPermissions || (ats.MayExec() && i.filemode().FileType() == linux.S_IFREG) {
-		if uint64(i.fs.clock.Now().Nanoseconds()) > i.attrTime {
+		if i.fs.clock.Now().After(i.attrTime) {
 			refreshed = true
 			if _, err := i.getAttr(ctx, i.fs.VFSFilesystem(), opts, 0, 0); err != nil {
 				return err
@@ -365,6 +374,10 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, d *kernfs.Dentr
 	return &fd.vfsfd, nil
 }
 
+func (i *inode) Valid(ctx context.Context) bool {
+	return i.entryTime.After(i.fs.clock.Now())
+}
+
 // Lookup implements kernfs.Inode.Lookup.
 func (i *inode) Lookup(ctx context.Context, name string) (kernfs.Inode, error) {
 	in := linux.FUSELookupIn{Name: linux.CString(name)}
@@ -508,7 +521,7 @@ func (i *inode) newEntry(ctx context.Context, name string, fileType linux.FileMo
 	if opcode != linux.FUSE_LOOKUP && ((out.Attr.Mode&linux.S_IFMT)^uint32(fileType) != 0 || out.NodeID == 0 || out.NodeID == linux.FUSE_ROOT_ID) {
 		return nil, linuxerr.EIO
 	}
-	child := i.fs.newInode(ctx, out.NodeID, out.Attr)
+	child := i.fs.newInode(ctx, out.NodeID, out.FUSEEntryOut)
 	if opcode == linux.FUSE_CREATE {
 		// File handler is returned by fuse server at a time of file create.
 		// Save it temporary in a created child, so Open could return it when invoked
@@ -544,7 +557,7 @@ func (i *inode) Readlink(ctx context.Context, mnt *vfs.Mount) (string, error) {
 		}
 		i.link = string(res.data[res.hdr.SizeBytes():])
 		if !mnt.Options().ReadOnly {
-			i.attrTime = 0
+			i.attrTime = time.ZeroTime
 		}
 	}
 	return i.link, nil
@@ -554,7 +567,7 @@ func (i *inode) Readlink(ctx context.Context, mnt *vfs.Mount) (string, error) {
 //
 // +checklocks:i.attrMu
 func (i *inode) getFUSEAttr() linux.FUSEAttr {
-	ns := time.Second.Nanoseconds()
+	ns := gotime.Second.Nanoseconds()
 	return linux.FUSEAttr{
 		Ino:       i.nodeID,
 		UID:       i.uid.Load(),
@@ -666,7 +679,7 @@ func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOp
 		return i.getFUSEAttr(), nil
 	}
 	i.fs.conn.mu.Unlock()
-	i.updateAttrs(out.Attr, out.AttrValid)
+	i.updateAttrs(out.Attr, int64(out.AttrValid), int64(out.AttrValidNsec))
 	return out.Attr, nil
 }
 
@@ -809,16 +822,16 @@ func (i *inode) setAttr(ctx context.Context, fs *vfs.Filesystem, creds *auth.Cre
 	if err := res.UnmarshalPayload(&out); err != nil {
 		return err
 	}
-	i.updateAttrs(out.Attr, out.AttrValid)
+	i.updateAttrs(out.Attr, int64(out.AttrValid), int64(out.AttrValidNsec))
 	return nil
 }
 
 // +checklocks:i.attrMu
-func (i *inode) updateAttrs(attr linux.FUSEAttr, attrTimeout uint64) {
+func (i *inode) updateAttrs(attr linux.FUSEAttr, validSec, validNSec int64) {
 	i.fs.conn.mu.Lock()
 	i.attrVersion.Store(i.fs.conn.attributeVersion.Add(1))
 	i.fs.conn.mu.Unlock()
-	i.attrTime = attrTimeout
+	i.attrTime = i.fs.clock.Now().AddTime(time.FromTimespec(linux.Timespec{Sec: validSec, Nsec: validNSec}))
 
 	i.ino.Store(attr.Ino)
 

pkg/sentry/fsimpl/kernfs/filesystem.go

@@ -113,16 +113,21 @@ func (fs *Filesystem) revalidateChildLocked(ctx context.Context, vfsObj *vfs.Vir
 	if child != nil {
 		// Cached dentry exists, revalidate.
 		if !child.inode.Valid(ctx) {
-			delete(parent.children, name)
-			if child.inode.Keep() {
-				// Drop the ref owned by kernfs.
-				fs.deferDecRef(child)
+			childInode, err := parent.inode.Lookup(ctx, name)
+			if err != nil {
+				delete(parent.children, child.name)
+				if child.inode.Keep() {
+					fs.deferDecRef(child)
+				}
+				rcs := vfsObj.InvalidateDentry(ctx, child.VFSDentry())
+				for _, rc := range rcs {
+					fs.deferDecRef(rc)
+				}
+				return nil, err
 			}
-			rcs := vfsObj.InvalidateDentry(ctx, child.VFSDentry())
-			for _, rc := range rcs {
-				fs.deferDecRef(rc)
-			}
-			child = nil
+			fs.deferDecRef(child.inode)
+			child.inode = childInode
+			return child, nil
 		}
 	}
 	if child == nil {

test/syscalls/BUILD

@@ -268,6 +268,7 @@ syscall_test(
 )
 
 syscall_test(
+    add_fusefs = True,
     test = "//test/syscalls/linux:fuse_test",
 )
 

test/syscalls/linux/BUILD

@@ -4652,6 +4652,7 @@ cc_binary(
         "//test/util:test_util",
         "//test/util:thread_util",
         "@com_google_absl//absl/flags:flag",
+        "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/synchronization",
     ],

test/syscalls/linux/fuse.cc

@@ -16,17 +16,22 @@
 #include <linux/capability.h>
 #include <stdio.h>
 #include <sys/mount.h>
+#include <sys/stat.h>
 #include <unistd.h>
 
+#include <cerrno>
 #include <cstdint>
-#include <cstring>
+#include <cstdlib>
 #include <string>
+#include <vector>
 
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "absl/strings/str_format.h"
-#include "test/util/capability_util.h"
+#include "absl/strings/string_view.h"
 #include "test/util/file_descriptor.h"
-#include "test/util/mount_util.h"
+#include "test/util/fs_util.h"
+#include "test/util/linux_capability_util.h"
 #include "test/util/posix_error.h"
 #include "test/util/temp_path.h"
 #include "test/util/test_util.h"
@@ -64,6 +69,24 @@ TEST(FuseTest, RejectBadInit) {
               SyscallFailsWithErrno(EINVAL));
 }
 
+TEST(FuseTest, LookupUpdatesInode) {
+  SKIP_IF(absl::NullSafeStringView(getenv("GVISOR_FUSE_TEST")) != "TRUE");
+  const std::string kFileData = "May thy knife chip and shatter.\n";
+  TempPath path = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
+      GetAbsoluteTestTmpdir(), kFileData, TempPath::kDefaultFileMode));
+
+  FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(path.path(), O_RDONLY));
+  std::vector<char> buf(kFileData.size());
+  ASSERT_THAT(ReadFd(fd.get(), buf.data(), kFileData.size()),
+              SyscallSucceedsWithValue(kFileData.size()));
+
+  ASSERT_THAT(unlink(JoinPath("/fuse", Basename(path.path())).c_str()),
+              SyscallSucceeds());
+
+  EXPECT_THAT(access(path.path().c_str(), O_RDONLY),
+              SyscallFailsWithErrno(ENOENT));
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace gvisor

test/syscalls/linux/open.cc

@@ -19,11 +19,13 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include <cstdlib>
 #include <memory>
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "absl/memory/memory.h"
+#include "absl/strings/string_view.h"
 #include "test/syscalls/linux/file_base.h"
 #include "test/util/capability_util.h"
 #include "test/util/cleanup.h"
@@ -319,6 +321,7 @@ TEST_F(OpenTest, AppendConcurrentWrite) {
   // externally, so we create a new inode each time when we open a file and we
   // can't guarantee that writes to files with O_APPEND will work correctly.
   SKIP_IF(getenv("GVISOR_GOFER_UNCACHED"));
+  SKIP_IF(absl::NullSafeStringView(getenv("GVISOR_FUSE_TEST")) == "TRUE");
 
   EXPECT_THAT(truncate(test_file_name_.c_str(), 0), SyscallSucceeds());