From 3e952d1e305e252766eccfd19b3a317cdf0c2c94 Mon Sep 17 00:00:00 2001 From: Lucas Manning Date: Thu, 4 Apr 2024 13:29:58 -0700 Subject: [PATCH] Modify FUSE inodes so they're not always assumed to be valid. PiperOrigin-RevId: 621954030 --- pkg/sentry/fsimpl/fuse/fusefs.go | 12 ++++++--- pkg/sentry/fsimpl/fuse/inode.go | 37 +++++++++++++++++--------- pkg/sentry/fsimpl/kernfs/filesystem.go | 23 +++++++++------- test/syscalls/BUILD | 1 + test/syscalls/linux/BUILD | 1 + test/syscalls/linux/fuse.cc | 29 +++++++++++++++++--- test/syscalls/linux/open.cc | 3 +++ 7 files changed, 78 insertions(+), 28 deletions(-) diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 97d25562a..3f266a2ce 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/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 diff --git a/pkg/sentry/fsimpl/fuse/inode.go b/pkg/sentry/fsimpl/fuse/inode.go index a3e4ee37c..bcf8bae59 100644 --- a/pkg/sentry/fsimpl/fuse/inode.go +++ b/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) diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index 622a85ece..5a5c72ff1 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/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 { diff --git a/test/syscalls/BUILD b/test/syscalls/BUILD index 6f5ebb8ce..23f464898 100644 --- a/test/syscalls/BUILD +++ b/test/syscalls/BUILD @@ -268,6 +268,7 @@ syscall_test( ) syscall_test( + add_fusefs = True, test = "//test/syscalls/linux:fuse_test", ) diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index a8d8b68b7..4cdc86b6a 100644 --- a/test/syscalls/linux/BUILD +++ b/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", ], diff --git a/test/syscalls/linux/fuse.cc b/test/syscalls/linux/fuse.cc index 8c67111e5..f24b0b895 100644 --- a/test/syscalls/linux/fuse.cc +++ b/test/syscalls/linux/fuse.cc @@ -16,17 +16,22 @@ #include #include #include +#include #include +#include #include -#include +#include #include +#include +#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 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 diff --git a/test/syscalls/linux/open.cc b/test/syscalls/linux/open.cc index 9e50cd8cb..8a21c2657 100644 --- a/test/syscalls/linux/open.cc +++ b/test/syscalls/linux/open.cc @@ -19,11 +19,13 @@ #include #include +#include #include #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());