From d6ee3ae6d797b7f66092971f395ef63f8a430c78 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 13 Aug 2020 22:23:40 +0000 Subject: Implement FUSE_LOOKUP Fixes #3231 Co-authored-by: Boyuan He --- pkg/abi/linux/fuse.go | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index 7e30483ee..346a9e6fc 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -14,12 +14,17 @@ package linux +import "gvisor.dev/gvisor/tools/go_marshal/marshal" + // +marshal type FUSEOpcode uint32 // +marshal type FUSEOpID uint64 +// FUSE_ROOT_ID is the id of root inode. +const FUSE_ROOT_ID = 1 + // Opcodes for FUSE operations. Analogous to the opcodes in include/linux/fuse.h. const ( FUSE_LOOKUP FUSEOpcode = 1 @@ -301,3 +306,54 @@ type FUSEGetAttrOut struct { // Attr contains the metadata returned from the FUSE server Attr FUSEAttr } + +// FUSEEntryOut is the reply sent by the daemon to the kernel +// for FUSE_MKNOD, FUSE_MKDIR, FUSE_SYMLINK, FUSE_LINK and +// FUSE_LOOKUP. +// +// +marshal +type FUSEEntryOut struct { + // NodeID is the ID for current inode. + NodeID uint64 + + // Generation is the generation number of inode. + // Used to identify an inode that have different ID at different time. + Generation uint64 + + // EntryValid indicates timeout for an entry. + EntryValid uint64 + + // AttrValid indicates timeout for an entry's attributes. + AttrValid uint64 + + // EntryValidNsec indicates timeout for an entry in nanosecond. + EntryValidNSec uint32 + + // AttrValidNsec indicates timeout for an entry's attributes in nanosecond. + AttrValidNSec uint32 + + // Attr contains the attributes of an entry. + Attr FUSEAttr +} + +// FUSELookupIn is the request sent by the kernel to the daemon +// to look up a file name. +// +// Dynamically-sized objects cannot be marshalled. +type FUSELookupIn struct { + marshal.StubMarshallable + + // Name is a file name to be looked up. + Name string +} + +// MarshalUnsafe serializes r.name to the dst buffer. +func (r *FUSELookupIn) MarshalUnsafe(buf []byte) { + copy(buf, []byte(r.Name)) +} + +// SizeBytes is the size of the memory representation of FUSELookupIn. +// 1 extra byte for null-terminated string. +func (r *FUSELookupIn) SizeBytes() int { + return len(r.Name) + 1 +} -- cgit v1.2.3 From 32044f94e9dfbb88c17d07b235b8ed5b07d2ff18 Mon Sep 17 00:00:00 2001 From: Boyuan He Date: Tue, 18 Aug 2020 01:46:39 +0000 Subject: Implement FUSE_OPEN/OPENDIR Fixes #3174 --- pkg/abi/linux/fuse.go | 38 +++++++++++ pkg/sentry/fsimpl/fuse/BUILD | 1 + pkg/sentry/fsimpl/fuse/connection.go | 16 ++++- pkg/sentry/fsimpl/fuse/file.go | 96 +++++++++++++++++++++++++++ pkg/sentry/fsimpl/fuse/fusefs.go | 97 +++++++++++++++++++++++++-- test/fuse/BUILD | 5 ++ test/fuse/linux/BUILD | 13 ++++ test/fuse/linux/fuse_base.cc | 50 +++++++------- test/fuse/linux/fuse_base.h | 9 +++ test/fuse/linux/open_test.cc | 124 +++++++++++++++++++++++++++++++++++ test/util/BUILD | 1 + test/util/fuse_util.cc | 59 +++++++++++++++++ test/util/fuse_util.h | 4 ++ 13 files changed, 481 insertions(+), 32 deletions(-) create mode 100644 pkg/sentry/fsimpl/fuse/file.go create mode 100644 test/fuse/linux/open_test.cc create mode 100644 test/util/fuse_util.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index 346a9e6fc..e09715ecd 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -357,3 +357,41 @@ func (r *FUSELookupIn) MarshalUnsafe(buf []byte) { func (r *FUSELookupIn) SizeBytes() int { return len(r.Name) + 1 } + +// MAX_NON_LFS indicates the maximum offset without large file support. +const MAX_NON_LFS = ((1 << 31) - 1) + +// flags returned by OPEN request. +const ( + // FOPEN_DIRECT_IO indicates bypassing page cache for this opened file. + FOPEN_DIRECT_IO = 1 << 0 + // FOPEN_KEEP_CACHE avoids invalidate of data cache on open. + FOPEN_KEEP_CACHE = 1 << 1 + // FOPEN_NONSEEKABLE indicates the file cannot be seeked. + FOPEN_NONSEEKABLE = 1 << 2 +) + +// FUSEOpenIn is the request sent by the kernel to the daemon, +// to negotiate flags and get file handle. +// +// +marshal +type FUSEOpenIn struct { + // Flags of this open request. + Flags uint32 + + _ uint32 +} + +// FUSEOpenOut is the reply sent by the daemon to the kernel +// for FUSEOpenIn. +// +// +marshal +type FUSEOpenOut struct { + // Fh is the file handler for opened file. + Fh uint64 + + // OpenFlag for the opened file. + OpenFlag uint32 + + _ uint32 +} diff --git a/pkg/sentry/fsimpl/fuse/BUILD b/pkg/sentry/fsimpl/fuse/BUILD index 999c16bfd..75a2e96ff 100644 --- a/pkg/sentry/fsimpl/fuse/BUILD +++ b/pkg/sentry/fsimpl/fuse/BUILD @@ -31,6 +31,7 @@ go_library( srcs = [ "connection.go", "dev.go", + "file.go", "fusefs.go", "init.go", "inode_refs.go", diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index eb1d1a2b7..9cfd67158 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -78,8 +78,13 @@ type Response struct { type connection struct { fd *DeviceFD + // mu protect access to struct memebers. + mu sync.Mutex + + // attributeVersion is the version of connection's attributes. + attributeVersion uint64 + // The following FUSE_INIT flags are currently unsupported by this implementation: - // - FUSE_ATOMIC_O_TRUNC: requires open(..., O_TRUNC) // - FUSE_EXPORT_SUPPORT // - FUSE_HANDLE_KILLPRIV // - FUSE_POSIX_LOCKS: requires POSIX locks @@ -113,6 +118,11 @@ type connection struct { // TODO(gvisor.dev/issue/3185): abort all queued requests. aborted bool + // atomicOTrunc is true when FUSE does not send a separate SETATTR request + // before open with O_TRUNC flag. + // Negotiated and only set in INIT. + atomicOTrunc bool + // connInitError if FUSE_INIT encountered error (major version mismatch). // Only set in INIT. connInitError bool @@ -189,6 +199,10 @@ type connection struct { // dontMask if filestestem does not apply umask to creation modes. // Negotiated in INIT. dontMask bool + + // noOpen if FUSE server doesn't support open operation. + // This flag only influence performance, not correctness of the program. + noOpen bool } // newFUSEConnection creates a FUSE connection to fd. diff --git a/pkg/sentry/fsimpl/fuse/file.go b/pkg/sentry/fsimpl/fuse/file.go new file mode 100644 index 000000000..ab60ab714 --- /dev/null +++ b/pkg/sentry/fsimpl/fuse/file.go @@ -0,0 +1,96 @@ +// 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. + +package fuse + +import ( + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" + "gvisor.dev/gvisor/pkg/sentry/vfs" + "gvisor.dev/gvisor/pkg/usermem" +) + +// fileDescription implements vfs.FileDescriptionImpl for fuse. +type fileDescription struct { + vfsfd vfs.FileDescription + vfs.FileDescriptionDefaultImpl + vfs.DentryMetadataFileDescriptionImpl + vfs.NoLockFD + + // the file handle used in userspace. + Fh uint64 + + // Nonseekable is indicate cannot perform seek on a file. + Nonseekable bool + + // DirectIO suggest fuse to use direct io operation. + DirectIO bool + + // OpenFlag is the flag returned by open. + OpenFlag uint32 +} + +func (fd *fileDescription) dentry() *kernfs.Dentry { + return fd.vfsfd.Dentry().Impl().(*kernfs.Dentry) +} + +func (fd *fileDescription) inode() *inode { + return fd.dentry().Inode().(*inode) +} + +func (fd *fileDescription) filesystem() *vfs.Filesystem { + return fd.vfsfd.VirtualDentry().Mount().Filesystem() +} + +// Release implements vfs.FileDescriptionImpl.Release. +func (fd *fileDescription) Release(ctx context.Context) {} + +// PRead implements vfs.FileDescriptionImpl.PRead. +func (fd *fileDescription) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts vfs.ReadOptions) (int64, error) { + return 0, nil +} + +// Read implements vfs.FileDescriptionImpl.Read. +func (fd *fileDescription) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.ReadOptions) (int64, error) { + return 0, nil +} + +// PWrite implements vfs.FileDescriptionImpl.PWrite. +func (fd *fileDescription) PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) { + return 0, nil +} + +// Write implements vfs.FileDescriptionImpl.Write. +func (fd *fileDescription) Write(ctx context.Context, src usermem.IOSequence, opts vfs.WriteOptions) (int64, error) { + return 0, nil +} + +// Seek implements vfs.FileDescriptionImpl.Seek. +func (fd *fileDescription) Seek(ctx context.Context, offset int64, whence int32) (int64, error) { + return 0, nil +} + +// Stat implements FileDescriptionImpl.Stat. +// Stat implements vfs.FileDescriptionImpl.Stat. +func (fd *fileDescription) Stat(ctx context.Context, opts vfs.StatOptions) (linux.Statx, error) { + fs := fd.filesystem() + inode := fd.inode() + return inode.Stat(ctx, fs, opts) +} + +// SetStat implements FileDescriptionImpl.SetStat. +func (fd *fileDescription) SetStat(ctx context.Context, opts vfs.SetStatOptions) error { + return nil +} diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index cee5acb3f..b5f05b80b 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -17,6 +17,7 @@ package fuse import ( "strconv" + "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -212,6 +213,18 @@ type inode struct { // the owning filesystem. fs is immutable. fs *filesystem + + // size of the file. + size uint64 + + // attributeVersion is the version of inode's attributes. + attributeVersion uint64 + + // attributeTime is the remaining vaild time of attributes. + attributeTime uint64 + + // version of the inode. + version uint64 } func (fs *filesystem) newRootInode(creds *auth.Credentials, mode linux.FileMode) *kernfs.Dentry { @@ -237,13 +250,87 @@ func (fs *filesystem) newInode(nodeID uint64, attr linux.FUSEAttr) *kernfs.Dentr // Open implements kernfs.Inode.Open. func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, vfsd *vfs.Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) { - fd, err := kernfs.NewGenericDirectoryFD(rp.Mount(), vfsd, &i.OrderedChildren, &i.locks, &opts, kernfs.GenericDirectoryFDOptions{ - SeekEnd: kernfs.SeekEndStaticEntries, - }) - if err != nil { + isDir := i.InodeAttrs.Mode().IsDir() + // return error if specified to open directory but inode is not a directory. + if !isDir && opts.Mode.IsDir() { + return nil, syserror.ENOTDIR + } + if opts.Flags&linux.O_LARGEFILE == 0 && atomic.LoadUint64(&i.size) > linux.MAX_NON_LFS { + return nil, syserror.EOVERFLOW + } + + // FOPEN_KEEP_CACHE is the defualt flag for noOpen. + fd := fileDescription{OpenFlag: linux.FOPEN_KEEP_CACHE} + // Only send open request when FUSE server support open or is opening a directory. + if !i.fs.conn.noOpen || isDir { + kernelTask := kernel.TaskFromContext(ctx) + if kernelTask == nil { + log.Warningf("fusefs.Inode.Open: couldn't get kernel task from context") + return nil, syserror.EINVAL + } + + var opcode linux.FUSEOpcode + if isDir { + opcode = linux.FUSE_OPENDIR + } else { + opcode = linux.FUSE_OPEN + } + in := linux.FUSEOpenIn{Flags: opts.Flags & ^uint32(linux.O_CREAT|linux.O_EXCL|linux.O_NOCTTY)} + if !i.fs.conn.atomicOTrunc { + in.Flags &= ^uint32(linux.O_TRUNC) + } + req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.NodeID, opcode, &in) + if err != nil { + return nil, err + } + + res, err := i.fs.conn.Call(kernelTask, req) + if err != nil { + return nil, err + } + if err := res.Error(); err == syserror.ENOSYS && !isDir { + i.fs.conn.noOpen = true + } else if err != nil { + return nil, err + } else { + out := linux.FUSEOpenOut{} + if err := res.UnmarshalPayload(&out); err != nil { + return nil, err + } + fd.OpenFlag = out.OpenFlag + fd.Fh = out.Fh + } + } + + if isDir { + fd.OpenFlag &= ^uint32(linux.FOPEN_DIRECT_IO) + } + + // TODO(gvisor.dev/issue/3234): invalidate mmap after implemented it for FUSE Inode + fd.DirectIO = fd.OpenFlag&linux.FOPEN_DIRECT_IO != 0 + fdOptions := &vfs.FileDescriptionOptions{} + if fd.OpenFlag&linux.FOPEN_NONSEEKABLE != 0 { + fdOptions.DenyPRead = true + fdOptions.DenyPWrite = true + fd.Nonseekable = true + } + + // If we don't send SETATTR before open (which is indicated by atomicOTrunc) + // and O_TRUNC is set, update the inode's version number and clean existing data + // by setting the file size to 0. + if i.fs.conn.atomicOTrunc && opts.Flags&linux.O_TRUNC != 0 { + i.fs.conn.mu.Lock() + i.fs.conn.attributeVersion++ + i.attributeVersion = i.fs.conn.attributeVersion + atomic.StoreUint64(&i.size, 0) + i.fs.conn.mu.Unlock() + i.attributeTime = 0 + } + + if err := fd.vfsfd.Init(&fd, opts.Flags, rp.Mount(), vfsd, fdOptions); err != nil { return nil, err } - return fd.VFSFileDescription(), nil + return &fd.vfsfd, nil } // Lookup implements kernfs.Inode.Lookup. diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 385920e17..209030ff1 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -6,3 +6,8 @@ syscall_test( fuse = "True", test = "//test/fuse/linux:stat_test", ) + +syscall_test( + fuse = "True", + test = "//test/fuse/linux:open_test", +) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index e4a614e11..1d989a2f4 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -18,6 +18,19 @@ cc_binary( ], ) +cc_binary( + name = "open_test", + testonly = 1, + srcs = ["open_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_library( name = "fuse_base", testonly = 1, diff --git a/test/fuse/linux/fuse_base.cc b/test/fuse/linux/fuse_base.cc index a9fe1044e..e734100b1 100644 --- a/test/fuse/linux/fuse_base.cc +++ b/test/fuse/linux/fuse_base.cc @@ -117,6 +117,16 @@ uint32_t FuseTest::GetServerTotalReceivedBytes() { static_cast(FuseTestCmd::kGetTotalReceivedBytes)); } +// Sends the `kSkipRequest` command to the FUSE server, which would skip +// current stored request data. +void FuseTest::SkipServerActualRequest() { + uint32_t cmd = static_cast(FuseTestCmd::kSkipRequest); + EXPECT_THAT(RetryEINTR(write)(sock_[0], &cmd, sizeof(cmd)), + SyscallSucceedsWithValue(sizeof(cmd))); + + WaitServerComplete(); +} + // Sends the `kSetInodeLookup` command, expected mode, and the path of the // inode to create under the mount point. void FuseTest::SetServerInodeLookup(const std::string& path, mode_t mode) { @@ -284,6 +294,9 @@ void FuseTest::ServerHandleCommand() { case FuseTestCmd::kGetNumUnsentResponses: ServerSendData(static_cast(responses_.RemainingBlocks())); break; + case FuseTestCmd::kSkipRequest: + ServerSkipReceivedRequest(); + break; default: FAIL() << "Unknown FuseTestCmd " << cmd; break; @@ -314,32 +327,7 @@ void FuseTest::ServerReceiveInodeLookup() { .len = out_len, .error = 0, }; - struct fuse_entry_out out_payload = { - .nodeid = nodeid_, - .generation = 0, - .entry_valid = 0, - .attr_valid = 0, - .entry_valid_nsec = 0, - .attr_valid_nsec = 0, - .attr = - (struct fuse_attr){ - .ino = nodeid_, - .size = 512, - .blocks = 4, - .atime = 0, - .mtime = 0, - .ctime = 0, - .atimensec = 0, - .mtimensec = 0, - .ctimensec = 0, - .mode = mode, - .nlink = 2, - .uid = 1234, - .gid = 4321, - .rdev = 12, - .blksize = 4096, - }, - }; + struct fuse_entry_out out_payload = DefaultEntryOut(mode, nodeid_); // Since this is only used in test, nodeid_ is simply increased by 1 to // comply with the unqiueness of different path. ++nodeid_; @@ -363,6 +351,15 @@ void FuseTest::ServerSendReceivedRequest() { SyscallSucceedsWithValue(mem_block.len)); } +// Skip the request pointed by current cursor. +void FuseTest::ServerSkipReceivedRequest() { + if (requests_.End()) { + FAIL() << "No more received request."; + return; + } + requests_.Next(); +} + // Handles FUSE request. Reads request from /dev/fuse, checks if it has the // same opcode as expected, and responds with the saved fake FUSE response. // The FUSE request is copied to the serial buffer and can be retrieved one- @@ -390,6 +387,7 @@ void FuseTest::ServerProcessFuseRequest() { requests_.AddMemBlock(in_header->opcode, buf.data(), len); + if (in_header->opcode == FUSE_RELEASE) return; // Check if there is a corresponding response. if (responses_.End()) { GTEST_NONFATAL_FAILURE_("No more FUSE response is expected"); diff --git a/test/fuse/linux/fuse_base.h b/test/fuse/linux/fuse_base.h index a21b4bb8d..2474b763f 100644 --- a/test/fuse/linux/fuse_base.h +++ b/test/fuse/linux/fuse_base.h @@ -42,6 +42,7 @@ enum class FuseTestCmd { kGetNumUnconsumedRequests, kGetNumUnsentResponses, kGetTotalReceivedBytes, + kSkipRequest, }; // Holds the information of a memory block in a serial buffer. @@ -158,6 +159,10 @@ class FuseTest : public ::testing::Test { // bytes from /dev/fuse. uint32_t GetServerTotalReceivedBytes(); + // Called by the testing thread to ask the FUSE server to skip stored + // request data. + void SkipServerActualRequest(); + protected: TempPath mount_point_; @@ -211,6 +216,10 @@ class FuseTest : public ::testing::Test { // Sends a uint32_t data via socket. inline void ServerSendData(uint32_t data); + // The FUSE server side's corresponding code of `SkipServerActualRequest()`. + // Handles `kSkipRequest` command. Skip the request pointed by current cursor. + void ServerSkipReceivedRequest(); + // Handles FUSE request sent to /dev/fuse by its saved responses. void ServerProcessFuseRequest(); diff --git a/test/fuse/linux/open_test.cc b/test/fuse/linux/open_test.cc new file mode 100644 index 000000000..ed0641587 --- /dev/null +++ b/test/fuse/linux/open_test.cc @@ -0,0 +1,124 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class OpenTest : public FuseTest { + protected: + const std::string test_file_ = "test_file"; + const mode_t regular_file_ = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO; + + struct fuse_open_out out_payload_ = { + .fh = 1, + .open_flags = O_RDWR, + }; +}; + +TEST_F(OpenTest, RegularFile) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + SetServerInodeLookup(test_file_, regular_file_); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_open_out), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload_); + SetServerResponse(FUSE_OPEN, iov_out); + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(test_file_path.c_str(), O_RDWR)); + + struct fuse_in_header in_header; + struct fuse_open_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_OPEN); + EXPECT_EQ(in_payload.flags, O_RDWR); + EXPECT_THAT(fcntl(fd.get(), F_GETFL), SyscallSucceedsWithValue(O_RDWR)); +} + +TEST_F(OpenTest, SetNoOpen) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + SetServerInodeLookup(test_file_, regular_file_); + + // ENOSYS indicates open is not implemented. + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_open_out), + .error = -ENOSYS, + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload_); + SetServerResponse(FUSE_OPEN, iov_out); + ASSERT_NO_ERRNO_AND_VALUE(Open(test_file_path.c_str(), O_RDWR)); + SkipServerActualRequest(); + + // check open doesn't send new request. + uint32_t recieved_before = GetServerTotalReceivedBytes(); + ASSERT_NO_ERRNO_AND_VALUE(Open(test_file_path.c_str(), O_RDWR)); + EXPECT_EQ(GetServerTotalReceivedBytes(), recieved_before); +} + +TEST_F(OpenTest, OpenFail) { + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_open_out), + .error = -ENOENT, + }; + + auto iov_out = FuseGenerateIovecs(out_header, out_payload_); + SetServerResponse(FUSE_OPENDIR, iov_out); + ASSERT_THAT(open(mount_point_.path().c_str(), O_RDWR), + SyscallFailsWithErrno(ENOENT)); + + struct fuse_in_header in_header; + struct fuse_open_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_OPENDIR); + EXPECT_EQ(in_payload.flags, O_RDWR); +} + +TEST_F(OpenTest, DirectoryFlagOnRegularFile) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + + SetServerInodeLookup(test_file_, regular_file_); + ASSERT_THAT(open(test_file_path.c_str(), O_RDWR | O_DIRECTORY), + SyscallFailsWithErrno(ENOTDIR)); +} + +} // namespace + +} // namespace testing +} // namespace gvisor diff --git a/test/util/BUILD b/test/util/BUILD index b0c2c2a5a..fc5fb3a8d 100644 --- a/test/util/BUILD +++ b/test/util/BUILD @@ -48,6 +48,7 @@ cc_library( cc_library( name = "fuse_util", testonly = 1, + srcs = ["fuse_util.cc"], hdrs = ["fuse_util.h"], ) diff --git a/test/util/fuse_util.cc b/test/util/fuse_util.cc new file mode 100644 index 000000000..5b10a9e45 --- /dev/null +++ b/test/util/fuse_util.cc @@ -0,0 +1,59 @@ +// 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 "test/util/fuse_util.h" + +#include +#include + +#include + +namespace gvisor { +namespace testing { + +// Create response body with specified mode and nodeID. +fuse_entry_out DefaultEntryOut(mode_t mode, uint64_t node_id) { + const int time_sec = 1595436289; + const int time_nsec = 134150844; + struct fuse_entry_out default_entry_out = { + .nodeid = node_id, + .generation = 0, + .entry_valid = time_sec, + .attr_valid = time_sec, + .entry_valid_nsec = time_nsec, + .attr_valid_nsec = time_nsec, + .attr = + (struct fuse_attr){ + .ino = node_id, + .size = 512, + .blocks = 4, + .atime = time_sec, + .mtime = time_sec, + .ctime = time_sec, + .atimensec = time_nsec, + .mtimensec = time_nsec, + .ctimensec = time_nsec, + .mode = mode, + .nlink = 2, + .uid = 1234, + .gid = 4321, + .rdev = 12, + .blksize = 4096, + }, + }; + return default_entry_out; +}; + +} // namespace testing +} // namespace gvisor diff --git a/test/util/fuse_util.h b/test/util/fuse_util.h index 5f5182b96..1f1bf64a4 100644 --- a/test/util/fuse_util.h +++ b/test/util/fuse_util.h @@ -15,6 +15,7 @@ #ifndef GVISOR_TEST_UTIL_FUSE_UTIL_H_ #define GVISOR_TEST_UTIL_FUSE_UTIL_H_ +#include #include #include @@ -62,6 +63,9 @@ std::vector FuseGenerateIovecs(T &first, Types &...args) { return first_iovec; } +// Return a fuse_entry_out FUSE server response body. +fuse_entry_out DefaultEntryOut(mode_t mode, uint64_t nodeId); + } // namespace testing } // namespace gvisor #endif // GVISOR_TEST_UTIL_FUSE_UTIL_H_ -- cgit v1.2.3 From 947088e10a15b5236f2af3206f67f27245ef2770 Mon Sep 17 00:00:00 2001 From: Boyuan He Date: Tue, 18 Aug 2020 20:59:28 +0000 Subject: Implement FUSE_RELEASE/RELEASEDIR Fixes #3314 --- pkg/abi/linux/fuse.go | 18 ++++++++++ pkg/sentry/fsimpl/fuse/dev.go | 6 ++++ pkg/sentry/fsimpl/fuse/file.go | 31 +++++++++++++++-- test/fuse/BUILD | 5 +++ test/fuse/linux/BUILD | 13 ++++++++ test/fuse/linux/fuse_base.cc | 3 +- test/fuse/linux/fuse_base.h | 6 ++-- test/fuse/linux/open_test.cc | 4 +++ test/fuse/linux/release_test.cc | 74 +++++++++++++++++++++++++++++++++++++++++ 9 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 test/fuse/linux/release_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index e09715ecd..a1b2a2abf 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -395,3 +395,21 @@ type FUSEOpenOut struct { _ uint32 } + +// FUSEReleaseIn is the request sent by the kernel to the daemon +// when there is no more reference to a file. +// +// +marshal +type FUSEReleaseIn struct { + // Fh is the file handler for the file to be released. + Fh uint64 + + // Flags of the file. + Flags uint32 + + // ReleaseFlags of this release request. + ReleaseFlags uint32 + + // LockOwner is the id of the lock owner if there is one. + LockOwner uint64 +} diff --git a/pkg/sentry/fsimpl/fuse/dev.go b/pkg/sentry/fsimpl/fuse/dev.go index 0efd2d90d..e2de8e097 100644 --- a/pkg/sentry/fsimpl/fuse/dev.go +++ b/pkg/sentry/fsimpl/fuse/dev.go @@ -168,6 +168,9 @@ func (fd *DeviceFD) readLocked(ctx context.Context, dst usermem.IOSequence, opts // We're done with this request. fd.queue.Remove(req) + if req.hdr.Opcode == linux.FUSE_RELEASE { + fd.numActiveRequests -= 1 + } // Restart the read as this request was invalid. log.Warningf("fuse.DeviceFD.Read: request found was too large. Restarting read.") @@ -184,6 +187,9 @@ func (fd *DeviceFD) readLocked(ctx context.Context, dst usermem.IOSequence, opts if readCursor >= req.hdr.Len { // Fully done with this req, remove it from the queue. fd.queue.Remove(req) + if req.hdr.Opcode == linux.FUSE_RELEASE { + fd.numActiveRequests -= 1 + } break } } diff --git a/pkg/sentry/fsimpl/fuse/file.go b/pkg/sentry/fsimpl/fuse/file.go index ab60ab714..01d20caf6 100644 --- a/pkg/sentry/fsimpl/fuse/file.go +++ b/pkg/sentry/fsimpl/fuse/file.go @@ -18,6 +18,8 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "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/vfs" "gvisor.dev/gvisor/pkg/usermem" ) @@ -54,8 +56,34 @@ func (fd *fileDescription) filesystem() *vfs.Filesystem { return fd.vfsfd.VirtualDentry().Mount().Filesystem() } +func (fd *fileDescription) statusFlags() uint32 { + return fd.vfsfd.StatusFlags() +} + // Release implements vfs.FileDescriptionImpl.Release. -func (fd *fileDescription) Release(ctx context.Context) {} +func (fd *fileDescription) Release(ctx context.Context) { + // no need to release if FUSE server doesn't implement Open. + conn := fd.inode().fs.conn + if conn.noOpen { + return + } + + in := linux.FUSEReleaseIn{ + Fh: fd.Fh, + Flags: fd.statusFlags(), + } + // TODO(gvisor.dev/issue/3245): add logic when we support file lock owner. + var opcode linux.FUSEOpcode + if fd.inode().Mode().IsDir() { + opcode = linux.FUSE_RELEASEDIR + } else { + opcode = linux.FUSE_RELEASE + } + kernelTask := kernel.TaskFromContext(ctx) + // ignoring errors and FUSE server reply is analogous to Linux's behavior. + req, _ := conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), fd.inode().NodeID, opcode, &in) + conn.CallAsync(kernelTask, req) +} // PRead implements vfs.FileDescriptionImpl.PRead. func (fd *fileDescription) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts vfs.ReadOptions) (int64, error) { @@ -82,7 +110,6 @@ func (fd *fileDescription) Seek(ctx context.Context, offset int64, whence int32) return 0, nil } -// Stat implements FileDescriptionImpl.Stat. // Stat implements vfs.FileDescriptionImpl.Stat. func (fd *fileDescription) Stat(ctx context.Context, opts vfs.StatOptions) (linux.Statx, error) { fs := fd.filesystem() diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 209030ff1..53cbadb3c 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -11,3 +11,8 @@ syscall_test( fuse = "True", test = "//test/fuse/linux:open_test", ) + +syscall_test( + fuse = "True", + test = "//test/fuse/linux:release_test", +) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index 1d989a2f4..abee1a33c 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -31,6 +31,19 @@ cc_binary( ], ) +cc_binary( + name = "release_test", + testonly = 1, + srcs = ["release_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_library( name = "fuse_base", testonly = 1, diff --git a/test/fuse/linux/fuse_base.cc b/test/fuse/linux/fuse_base.cc index e734100b1..98b4e1466 100644 --- a/test/fuse/linux/fuse_base.cc +++ b/test/fuse/linux/fuse_base.cc @@ -387,7 +387,8 @@ void FuseTest::ServerProcessFuseRequest() { requests_.AddMemBlock(in_header->opcode, buf.data(), len); - if (in_header->opcode == FUSE_RELEASE) return; + if (in_header->opcode == FUSE_RELEASE || in_header->opcode == FUSE_RELEASEDIR) + return; // Check if there is a corresponding response. if (responses_.End()) { GTEST_NONFATAL_FAILURE_("No more FUSE response is expected"); diff --git a/test/fuse/linux/fuse_base.h b/test/fuse/linux/fuse_base.h index 2474b763f..ff4c4499d 100644 --- a/test/fuse/linux/fuse_base.h +++ b/test/fuse/linux/fuse_base.h @@ -166,13 +166,13 @@ class FuseTest : public ::testing::Test { protected: TempPath mount_point_; + // Unmounts the mountpoint of the FUSE server. + void UnmountFuse(); + private: // Opens /dev/fuse and inherit the file descriptor for the FUSE server. void MountFuse(); - // Unmounts the mountpoint of the FUSE server. - void UnmountFuse(); - // Creates a socketpair for communication and forks FUSE server. void SetUpFuseServer(); diff --git a/test/fuse/linux/open_test.cc b/test/fuse/linux/open_test.cc index ed0641587..4b0c4a805 100644 --- a/test/fuse/linux/open_test.cc +++ b/test/fuse/linux/open_test.cc @@ -33,6 +33,10 @@ namespace testing { namespace { class OpenTest : public FuseTest { + // OpenTest doesn't care the release request when close a fd, + // so doesn't check leftover requests when tearing down. + void TearDown() { UnmountFuse(); } + protected: const std::string test_file_ = "test_file"; const mode_t regular_file_ = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO; diff --git a/test/fuse/linux/release_test.cc b/test/fuse/linux/release_test.cc new file mode 100644 index 000000000..b5adb0870 --- /dev/null +++ b/test/fuse/linux/release_test.cc @@ -0,0 +1,74 @@ +// 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 +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class ReleaseTest : public FuseTest { + protected: + const std::string test_file_ = "test_file"; +}; + +TEST_F(ReleaseTest, RegularFile) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + SetServerInodeLookup(test_file_, S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_open_out), + }; + struct fuse_open_out out_payload = { + .fh = 1, + .open_flags = O_RDWR, + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_OPEN, iov_out); + FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(test_file_path, O_RDWR)); + SkipServerActualRequest(); + ASSERT_THAT(close(fd.release()), SyscallSucceeds()); + + 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_RELEASE); + EXPECT_EQ(in_payload.flags, O_RDWR); + EXPECT_EQ(in_payload.fh, 1); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From b53e10f391929e4ad9345a823a0cf33bcfedf413 Mon Sep 17 00:00:00 2001 From: Boyuan He Date: Tue, 18 Aug 2020 21:51:06 +0000 Subject: Implement FUSE_MKNOD Fixes #3492 --- pkg/abi/linux/fuse.go | 43 ++++++++++++++++ pkg/sentry/fsimpl/fuse/fusefs.go | 13 +++++ test/fuse/BUILD | 5 ++ test/fuse/linux/BUILD | 14 +++++ test/fuse/linux/mknod_test.cc | 107 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 182 insertions(+) create mode 100644 test/fuse/linux/mknod_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index a1b2a2abf..97d960096 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -413,3 +413,46 @@ type FUSEReleaseIn struct { // LockOwner is the id of the lock owner if there is one. LockOwner uint64 } + +// FUSEMknodMeta contains all the static fields of FUSEMknodIn, +// which is used for FUSE_MKNOD. +// +// +marshal +type FUSEMknodMeta struct { + // Mode of the inode to create. + Mode uint32 + + // Rdev encodes device major and minor information. + Rdev uint32 + + // Umask is the current file mode creation mask. + Umask uint32 + + _ uint32 +} + +// FUSEMknodIn contains all the arguments sent by the kernel +// to the daemon, to create a new file node. +// +// Dynamically-sized objects cannot be marshalled. +type FUSEMknodIn struct { + marshal.StubMarshallable + + // MknodMeta contains mode, rdev and umash field for FUSE_MKNODS. + MknodMeta FUSEMknodMeta + + // Name is the name of the node to create. + Name string +} + +// MarshalUnsafe serializes r.MknodMeta and r.Name to the dst buffer. +func (r *FUSEMknodIn) MarshalUnsafe(buf []byte) { + r.MknodMeta.MarshalUnsafe(buf[:r.MknodMeta.SizeBytes()]) + copy(buf[r.MknodMeta.SizeBytes():], r.Name) +} + +// SizeBytes is the size of the memory representation of FUSEMknodIn. +// 1 extra byte for null-terminated string. +func (r *FUSEMknodIn) SizeBytes() int { + return r.MknodMeta.SizeBytes() + len(r.Name) + 1 +} diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index b5f05b80b..5cef0b94f 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -349,6 +349,19 @@ func (inode) Valid(ctx context.Context) bool { return true } +// NewNode implements kernfs.Inode.NewNode. +func (i *inode) NewNode(ctx context.Context, name string, opts vfs.MknodOptions) (*vfs.Dentry, error) { + in := linux.FUSEMknodIn{ + MknodMeta: linux.FUSEMknodMeta{ + Mode: uint32(opts.Mode), + Rdev: linux.MakeDeviceID(uint16(opts.DevMajor), opts.DevMinor), + Umask: uint32(kernel.TaskFromContext(ctx).FSContext().Umask()), + }, + Name: name, + } + return i.newEntry(ctx, name, opts.Mode.FileType(), linux.FUSE_MKNOD, &in) +} + // newEntry calls FUSE server for entry creation and allocates corresponding entry according to response. // Shared by FUSE_MKNOD, FUSE_MKDIR, FUSE_SYMLINK, FUSE_LINK and FUSE_LOOKUP. func (i *inode) newEntry(ctx context.Context, name string, fileType linux.FileMode, opcode linux.FUSEOpcode, payload marshal.Marshallable) (*vfs.Dentry, error) { diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 53cbadb3c..ff2948eb3 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -16,3 +16,8 @@ syscall_test( fuse = "True", test = "//test/fuse/linux:release_test", ) + +syscall_test( + fuse = "True", + test = "//test/fuse/linux:mknod_test", +) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index abee1a33c..c0f917606 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -44,6 +44,20 @@ cc_binary( ], ) +cc_binary( + name = "mknod_test", + testonly = 1, + srcs = ["mknod_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:temp_umask", + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_library( name = "fuse_base", testonly = 1, diff --git a/test/fuse/linux/mknod_test.cc b/test/fuse/linux/mknod_test.cc new file mode 100644 index 000000000..74c74d76b --- /dev/null +++ b/test/fuse/linux/mknod_test.cc @@ -0,0 +1,107 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/temp_umask.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class MknodTest : public FuseTest { + protected: + const std::string test_file_ = "test_file"; + const mode_t perms_ = S_IRWXU | S_IRWXG | S_IRWXO; +}; + +TEST_F(MknodTest, RegularFile) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + const mode_t new_umask = 0077; + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out), + }; + struct fuse_entry_out out_payload = DefaultEntryOut(S_IFREG | perms_, 5); + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_MKNOD, iov_out); + TempUmask mask(new_umask); + ASSERT_THAT(mknod(test_file_path.c_str(), perms_, 0), SyscallSucceeds()); + + struct fuse_in_header in_header; + struct fuse_mknod_in in_payload; + std::vector actual_file(test_file_.length() + 1); + auto iov_in = FuseGenerateIovecs(in_header, in_payload, actual_file); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_header.len, + sizeof(in_header) + sizeof(in_payload) + test_file_.length() + 1); + EXPECT_EQ(in_header.opcode, FUSE_MKNOD); + EXPECT_EQ(in_payload.mode & 0777, perms_ & ~new_umask); + EXPECT_EQ(in_payload.umask, new_umask); + EXPECT_EQ(in_payload.rdev, 0); + EXPECT_EQ(std::string(actual_file.data()), test_file_); +} + +TEST_F(MknodTest, FileTypeError) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out), + }; + // server return directory instead of regular file should cause an error. + struct fuse_entry_out out_payload = DefaultEntryOut(S_IFDIR | perms_, 5); + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_MKNOD, iov_out); + ASSERT_THAT(mknod(test_file_path.c_str(), perms_, 0), + SyscallFailsWithErrno(EIO)); + SkipServerActualRequest(); +} + +TEST_F(MknodTest, NodeIDError) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out), + }; + struct fuse_entry_out out_payload = + DefaultEntryOut(S_IFREG | perms_, FUSE_ROOT_ID); + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_MKNOD, iov_out); + ASSERT_THAT(mknod(test_file_path.c_str(), perms_, 0), + SyscallFailsWithErrno(EIO)); + SkipServerActualRequest(); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From b50c03b5715905ebd82b1006c1bb2e2d4eb9334d Mon Sep 17 00:00:00 2001 From: Boyuan He Date: Tue, 18 Aug 2020 23:09:34 +0000 Subject: Implement FUSE_SYMLINK Fixes #3452 --- pkg/abi/linux/fuse.go | 27 ++++++++++++ pkg/sentry/fsimpl/fuse/fusefs.go | 9 ++++ test/fuse/BUILD | 5 +++ test/fuse/linux/BUILD | 13 ++++++ test/fuse/linux/symlink_test.cc | 88 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 142 insertions(+) create mode 100644 test/fuse/linux/symlink_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index 97d960096..ea5a7fd43 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -456,3 +456,30 @@ func (r *FUSEMknodIn) MarshalUnsafe(buf []byte) { func (r *FUSEMknodIn) SizeBytes() int { return r.MknodMeta.SizeBytes() + len(r.Name) + 1 } + +// FUSESymLinkIn is the request sent by the kernel to the daemon, +// to create a symbolic link. +// +// Dynamically-sized objects cannot be marshalled. +type FUSESymLinkIn struct { + marshal.StubMarshallable + + // Name of symlink to create. + Name string + + // Target of the symlink. + Target string +} + +// MarshalUnsafe serializes r.Name and r.Target to the dst buffer. +// Left null-termination at end of r.Name and r.Target. +func (r *FUSESymLinkIn) MarshalUnsafe(buf []byte) { + copy(buf, r.Name) + copy(buf[len(r.Name)+1:], r.Target) +} + +// SizeBytes is the size of the memory representation of FUSESymLinkIn. +// 2 extra bytes for null-terminated string. +func (r *FUSESymLinkIn) SizeBytes() int { + return len(r.Name) + len(r.Target) + 2 +} diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 5cef0b94f..0021e2933 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -362,6 +362,15 @@ func (i *inode) NewNode(ctx context.Context, name string, opts vfs.MknodOptions) return i.newEntry(ctx, name, opts.Mode.FileType(), linux.FUSE_MKNOD, &in) } +// NewSymlink implements kernfs.Inode.NewSymlink. +func (i *inode) NewSymlink(ctx context.Context, name, target string) (*vfs.Dentry, error) { + in := linux.FUSESymLinkIn{ + Name: name, + Target: target, + } + return i.newEntry(ctx, name, linux.S_IFLNK, linux.FUSE_SYMLINK, &in) +} + // newEntry calls FUSE server for entry creation and allocates corresponding entry according to response. // Shared by FUSE_MKNOD, FUSE_MKDIR, FUSE_SYMLINK, FUSE_LINK and FUSE_LOOKUP. func (i *inode) newEntry(ctx context.Context, name string, fileType linux.FileMode, opcode linux.FUSEOpcode, payload marshal.Marshallable) (*vfs.Dentry, error) { diff --git a/test/fuse/BUILD b/test/fuse/BUILD index ff2948eb3..2f91fe2c7 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -21,3 +21,8 @@ syscall_test( fuse = "True", test = "//test/fuse/linux:mknod_test", ) + +syscall_test( + fuse = "True", + test = "//test/fuse/linux:symlink_test", +) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index c0f917606..df42857f6 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -58,6 +58,19 @@ cc_binary( ], ) +cc_binary( + name = "symlink_test", + testonly = 1, + srcs = ["symlink_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_library( name = "fuse_base", testonly = 1, diff --git a/test/fuse/linux/symlink_test.cc b/test/fuse/linux/symlink_test.cc new file mode 100644 index 000000000..2c3a52987 --- /dev/null +++ b/test/fuse/linux/symlink_test.cc @@ -0,0 +1,88 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class SymlinkTest : public FuseTest { + protected: + const std::string target_file_ = "target_file_"; + const std::string symlink_ = "symlink_"; + const mode_t perms_ = S_IRWXU | S_IRWXG | S_IRWXO; +}; + +TEST_F(SymlinkTest, CreateSymLink) { + const std::string symlink_path = + JoinPath(mount_point_.path().c_str(), symlink_); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out), + }; + struct fuse_entry_out out_payload = DefaultEntryOut(S_IFLNK | perms_, 5); + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SYMLINK, iov_out); + ASSERT_THAT(symlink(target_file_.c_str(), symlink_path.c_str()), + SyscallSucceeds()); + + struct fuse_in_header in_header; + std::vector actual_target_file(target_file_.length() + 1); + std::vector actual_symlink(symlink_.length() + 1); + auto iov_in = + FuseGenerateIovecs(in_header, actual_symlink, actual_target_file); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_header.len, + sizeof(in_header) + symlink_.length() + target_file_.length() + 2); + EXPECT_EQ(in_header.opcode, FUSE_SYMLINK); + EXPECT_EQ(std::string(actual_target_file.data()), target_file_); + EXPECT_EQ(std::string(actual_symlink.data()), symlink_); +} + +TEST_F(SymlinkTest, FileTypeError) { + const std::string symlink_path = + JoinPath(mount_point_.path().c_str(), symlink_); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out), + }; + struct fuse_entry_out out_payload = DefaultEntryOut(S_IFREG | perms_, 5); + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SYMLINK, iov_out); + ASSERT_THAT(symlink(target_file_.c_str(), symlink_path.c_str()), + SyscallFailsWithErrno(EIO)); + SkipServerActualRequest(); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From 733d013f979a2107fd866ff3c05249c3e8ba5102 Mon Sep 17 00:00:00 2001 From: Boyuan He Date: Tue, 18 Aug 2020 23:50:22 +0000 Subject: Implement FUSE_READLINK Fixes #3316 --- pkg/abi/linux/fuse.go | 11 ++++ pkg/sentry/fsimpl/fuse/fusefs.go | 30 ++++++++++ pkg/sentry/fsimpl/kernfs/filesystem.go | 2 +- pkg/sentry/fsimpl/kernfs/inode_impl_util.go | 2 +- pkg/sentry/fsimpl/kernfs/kernfs.go | 2 +- pkg/sentry/fsimpl/kernfs/symlink.go | 2 +- pkg/sentry/fsimpl/proc/task_fds.go | 2 +- pkg/sentry/fsimpl/proc/task_files.go | 6 +- pkg/sentry/fsimpl/proc/tasks_files.go | 12 ++-- test/fuse/BUILD | 5 ++ test/fuse/linux/BUILD | 13 +++++ test/fuse/linux/readlink_test.cc | 85 +++++++++++++++++++++++++++++ 12 files changed, 158 insertions(+), 14 deletions(-) create mode 100644 test/fuse/linux/readlink_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index ea5a7fd43..5de1433d7 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -483,3 +483,14 @@ func (r *FUSESymLinkIn) MarshalUnsafe(buf []byte) { func (r *FUSESymLinkIn) SizeBytes() int { return len(r.Name) + len(r.Target) + 2 } + +// FUSEEmptyIn is used by operations without request body. +type FUSEEmptyIn struct{ marshal.StubMarshallable } + +// MarshalUnsafe do nothing for marshal. +func (r *FUSEEmptyIn) MarshalUnsafe(buf []byte) {} + +// SizeBytes is 0 for empty request. +func (r *FUSEEmptyIn) SizeBytes() int { + return 0 +} diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 0021e2933..8db337a2e 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -225,6 +225,9 @@ type inode struct { // version of the inode. version uint64 + + // link is result of following a symbolic link. + link string } func (fs *filesystem) newRootInode(creds *auth.Credentials, mode linux.FileMode) *kernfs.Dentry { @@ -406,6 +409,33 @@ func (i *inode) newEntry(ctx context.Context, name string, fileType linux.FileMo return child.VFSDentry(), nil } +// Readlink implements kernfs.Inode.Readlink. +func (i *inode) Readlink(ctx context.Context, mnt *vfs.Mount) (string, error) { + if i.Mode().FileType()&linux.S_IFLNK == 0 { + return "", syserror.EINVAL + } + if i.link == "" { + kernelTask := kernel.TaskFromContext(ctx) + if kernelTask == nil { + log.Warningf("fusefs.Inode.Readlink: couldn't get kernel task from context") + return "", syserror.EINVAL + } + req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.NodeID, linux.FUSE_READLINK, &linux.FUSEEmptyIn{}) + if err != nil { + return "", err + } + res, err := i.fs.conn.Call(kernelTask, req) + if err != nil { + return "", err + } + i.link = string(res.data[res.hdr.SizeBytes():]) + if !mnt.Options().ReadOnly { + i.attributeTime = 0 + } + } + return i.link, nil +} + // statFromFUSEAttr makes attributes from linux.FUSEAttr to linux.Statx. The // opts.Sync attribute is ignored since the synchronization is handled by the // FUSE server. diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index 904203070..7aaf1146d 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/pkg/sentry/fsimpl/kernfs/filesystem.go @@ -548,7 +548,7 @@ func (fs *Filesystem) ReadlinkAt(ctx context.Context, rp *vfs.ResolvingPath) (st if !d.Impl().(*Dentry).isSymlink() { return "", syserror.EINVAL } - return inode.Readlink(ctx) + return inode.Readlink(ctx, rp.Mount()) } // RenameAt implements vfs.FilesystemImpl.RenameAt. diff --git a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go index 74408e322..53758a4b8 100644 --- a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go +++ b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go @@ -172,7 +172,7 @@ func (InodeNoDynamicLookup) Valid(ctx context.Context) bool { type InodeNotSymlink struct{} // Readlink implements Inode.Readlink. -func (InodeNotSymlink) Readlink(context.Context) (string, error) { +func (InodeNotSymlink) Readlink(context.Context, *vfs.Mount) (string, error) { return "", syserror.EINVAL } diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index 67a0347fe..f656e2a8b 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -437,7 +437,7 @@ type inodeDynamicLookup interface { type inodeSymlink interface { // Readlink returns the target of a symbolic link. If an inode is not a // symlink, the implementation should return EINVAL. - Readlink(ctx context.Context) (string, error) + Readlink(ctx context.Context, mnt *vfs.Mount) (string, error) // Getlink returns the target of a symbolic link, as used by path // resolution: diff --git a/pkg/sentry/fsimpl/kernfs/symlink.go b/pkg/sentry/fsimpl/kernfs/symlink.go index 64731a3e4..a9812fcef 100644 --- a/pkg/sentry/fsimpl/kernfs/symlink.go +++ b/pkg/sentry/fsimpl/kernfs/symlink.go @@ -52,7 +52,7 @@ func (s *StaticSymlink) Init(creds *auth.Credentials, devMajor uint32, devMinor } // Readlink implements Inode. -func (s *StaticSymlink) Readlink(_ context.Context) (string, error) { +func (s *StaticSymlink) Readlink(_ context.Context, _ *vfs.Mount) (string, error) { return s.target, nil } diff --git a/pkg/sentry/fsimpl/proc/task_fds.go b/pkg/sentry/fsimpl/proc/task_fds.go index 94ec2ff69..47d744a57 100644 --- a/pkg/sentry/fsimpl/proc/task_fds.go +++ b/pkg/sentry/fsimpl/proc/task_fds.go @@ -214,7 +214,7 @@ func (fs *filesystem) newFDSymlink(task *kernel.Task, fd int32, ino uint64) *ker return d } -func (s *fdSymlink) Readlink(ctx context.Context) (string, error) { +func (s *fdSymlink) Readlink(ctx context.Context, _ *vfs.Mount) (string, error) { file, _ := getTaskFD(s.task, s.fd) if file == nil { return "", syserror.ENOENT diff --git a/pkg/sentry/fsimpl/proc/task_files.go b/pkg/sentry/fsimpl/proc/task_files.go index ce87b0d47..87726d8f7 100644 --- a/pkg/sentry/fsimpl/proc/task_files.go +++ b/pkg/sentry/fsimpl/proc/task_files.go @@ -668,7 +668,7 @@ func (fs *filesystem) newExeSymlink(task *kernel.Task, ino uint64) *kernfs.Dentr } // Readlink implements kernfs.Inode. -func (s *exeSymlink) Readlink(ctx context.Context) (string, error) { +func (s *exeSymlink) Readlink(ctx context.Context, _ *vfs.Mount) (string, error) { if !kernel.ContextCanTrace(ctx, s.task, false) { return "", syserror.EACCES } @@ -808,11 +808,11 @@ func (fs *filesystem) newNamespaceSymlink(task *kernel.Task, ino uint64, ns stri } // Readlink implements Inode. -func (s *namespaceSymlink) Readlink(ctx context.Context) (string, error) { +func (s *namespaceSymlink) Readlink(ctx context.Context, mnt *vfs.Mount) (string, error) { if err := checkTaskState(s.task); err != nil { return "", err } - return s.StaticSymlink.Readlink(ctx) + return s.StaticSymlink.Readlink(ctx, mnt) } // Getlink implements Inode.Getlink. diff --git a/pkg/sentry/fsimpl/proc/tasks_files.go b/pkg/sentry/fsimpl/proc/tasks_files.go index 8c41729e4..68c541046 100644 --- a/pkg/sentry/fsimpl/proc/tasks_files.go +++ b/pkg/sentry/fsimpl/proc/tasks_files.go @@ -51,7 +51,7 @@ func (fs *filesystem) newSelfSymlink(creds *auth.Credentials, ino uint64, pidns return d } -func (s *selfSymlink) Readlink(ctx context.Context) (string, error) { +func (s *selfSymlink) Readlink(ctx context.Context, _ *vfs.Mount) (string, error) { t := kernel.TaskFromContext(ctx) if t == nil { // Who is reading this link? @@ -64,8 +64,8 @@ func (s *selfSymlink) Readlink(ctx context.Context) (string, error) { return strconv.FormatUint(uint64(tgid), 10), nil } -func (s *selfSymlink) Getlink(ctx context.Context, _ *vfs.Mount) (vfs.VirtualDentry, string, error) { - target, err := s.Readlink(ctx) +func (s *selfSymlink) Getlink(ctx context.Context, mnt *vfs.Mount) (vfs.VirtualDentry, string, error) { + target, err := s.Readlink(ctx, mnt) return vfs.VirtualDentry{}, target, err } @@ -94,7 +94,7 @@ func (fs *filesystem) newThreadSelfSymlink(creds *auth.Credentials, ino uint64, return d } -func (s *threadSelfSymlink) Readlink(ctx context.Context) (string, error) { +func (s *threadSelfSymlink) Readlink(ctx context.Context, _ *vfs.Mount) (string, error) { t := kernel.TaskFromContext(ctx) if t == nil { // Who is reading this link? @@ -108,8 +108,8 @@ func (s *threadSelfSymlink) Readlink(ctx context.Context) (string, error) { return fmt.Sprintf("%d/task/%d", tgid, tid), nil } -func (s *threadSelfSymlink) Getlink(ctx context.Context, _ *vfs.Mount) (vfs.VirtualDentry, string, error) { - target, err := s.Readlink(ctx) +func (s *threadSelfSymlink) Getlink(ctx context.Context, mnt *vfs.Mount) (vfs.VirtualDentry, string, error) { + target, err := s.Readlink(ctx, mnt) return vfs.VirtualDentry{}, target, err } diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 2f91fe2c7..c2bdcf1ba 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -26,3 +26,8 @@ syscall_test( fuse = "True", test = "//test/fuse/linux:symlink_test", ) + +syscall_test( + fuse = "True", + test = "//test/fuse/linux:readlink_test", +) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index df42857f6..d3e8ca148 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -71,6 +71,19 @@ cc_binary( ], ) +cc_binary( + name = "readlink_test", + testonly = 1, + srcs = ["readlink_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_library( name = "fuse_base", testonly = 1, diff --git a/test/fuse/linux/readlink_test.cc b/test/fuse/linux/readlink_test.cc new file mode 100644 index 000000000..2cba8fc23 --- /dev/null +++ b/test/fuse/linux/readlink_test.cc @@ -0,0 +1,85 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class ReadlinkTest : public FuseTest { + protected: + const std::string test_file_ = "test_file_"; + const mode_t perms_ = S_IRWXU | S_IRWXG | S_IRWXO; +}; + +TEST_F(ReadlinkTest, ReadSymLink) { + const std::string symlink_path = + JoinPath(mount_point_.path().c_str(), test_file_); + SetServerInodeLookup(test_file_, S_IFLNK | perms_); + + struct fuse_out_header out_header = { + .len = static_cast(sizeof(struct fuse_out_header)) + + static_cast(test_file_.length()) + 1, + }; + std::string link = test_file_; + auto iov_out = FuseGenerateIovecs(out_header, link); + SetServerResponse(FUSE_READLINK, iov_out); + const std::string actual_link = + ASSERT_NO_ERRNO_AND_VALUE(ReadLink(symlink_path)); + + struct fuse_in_header in_header; + auto iov_in = FuseGenerateIovecs(in_header); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_header.len, sizeof(in_header)); + EXPECT_EQ(in_header.opcode, FUSE_READLINK); + EXPECT_EQ(0, memcmp(actual_link.c_str(), link.data(), link.size())); + + // next readlink should have link cached, so shouldn't have new request to + // server. + uint32_t recieved_before = GetServerTotalReceivedBytes(); + ASSERT_NO_ERRNO(ReadLink(symlink_path)); + EXPECT_EQ(GetServerTotalReceivedBytes(), recieved_before); +} + +TEST_F(ReadlinkTest, NotSymlink) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + SetServerInodeLookup(test_file_, S_IFREG | perms_); + + std::vector buf(PATH_MAX + 1); + ASSERT_THAT(readlink(test_file_path.c_str(), buf.data(), PATH_MAX), + SyscallFailsWithErrno(EINVAL)); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From 4d26c9929de31cdfe3551d4b8be90a07f98fed55 Mon Sep 17 00:00:00 2001 From: Boyuan He Date: Tue, 18 Aug 2020 22:45:47 +0000 Subject: Implement FUSE_MKDIR Fixes #3392 --- pkg/abi/linux/fuse.go | 38 ++++++++++++++++ pkg/sentry/fsimpl/fuse/BUILD | 1 + pkg/sentry/fsimpl/fuse/directory.go | 51 +++++++++++++++++++++ pkg/sentry/fsimpl/fuse/fusefs.go | 27 ++++++++++-- test/fuse/BUILD | 5 +++ test/fuse/linux/BUILD | 14 ++++++ test/fuse/linux/mkdir_test.cc | 88 +++++++++++++++++++++++++++++++++++++ 7 files changed, 221 insertions(+), 3 deletions(-) create mode 100644 pkg/sentry/fsimpl/fuse/directory.go create mode 100644 test/fuse/linux/mkdir_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index 5de1433d7..4ef0ab9a7 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -494,3 +494,41 @@ func (r *FUSEEmptyIn) MarshalUnsafe(buf []byte) {} func (r *FUSEEmptyIn) SizeBytes() int { return 0 } + +// FUSEMkdirMeta contains all the static fields of FUSEMkdirIn, +// which is used for FUSE_MKDIR. +// +// +marshal +type FUSEMkdirMeta struct { + // Mode of the directory of create. + Mode uint32 + + // Umask is the user file creation mask. + Umask uint32 +} + +// FUSEMkdirIn contains all the arguments sent by the kernel +// to the daemon, to create a new directory. +// +// Dynamically-sized objects cannot be marshalled. +type FUSEMkdirIn struct { + marshal.StubMarshallable + + // MkdirMeta contains Mode and Umask of the directory to create. + MkdirMeta FUSEMkdirMeta + + // Name of the directory to create. + Name string +} + +// MarshalUnsafe serializes r.MkdirMeta and r.Name to the dst buffer. +func (r *FUSEMkdirIn) MarshalUnsafe(buf []byte) { + r.MkdirMeta.MarshalUnsafe(buf[:r.MkdirMeta.SizeBytes()]) + copy(buf[r.MkdirMeta.SizeBytes():], r.Name) +} + +// SizeBytes is the size of the memory representation of FUSEMkdirIn. +// 1 extra byte for null-terminated Name string. +func (r *FUSEMkdirIn) SizeBytes() int { + return r.MkdirMeta.SizeBytes() + len(r.Name) + 1 +} diff --git a/pkg/sentry/fsimpl/fuse/BUILD b/pkg/sentry/fsimpl/fuse/BUILD index 75a2e96ff..b9dcee96a 100644 --- a/pkg/sentry/fsimpl/fuse/BUILD +++ b/pkg/sentry/fsimpl/fuse/BUILD @@ -31,6 +31,7 @@ go_library( srcs = [ "connection.go", "dev.go", + "directory.go", "file.go", "fusefs.go", "init.go", diff --git a/pkg/sentry/fsimpl/fuse/directory.go b/pkg/sentry/fsimpl/fuse/directory.go new file mode 100644 index 000000000..44d41712a --- /dev/null +++ b/pkg/sentry/fsimpl/fuse/directory.go @@ -0,0 +1,51 @@ +// 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. + +package fuse + +import ( + "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/sentry/vfs" + "gvisor.dev/gvisor/pkg/syserror" + "gvisor.dev/gvisor/pkg/usermem" +) + +type directoryFD struct { + fileDescription +} + +// Allocate implements directoryFD.Allocate. +func (directoryFD) Allocate(ctx context.Context, mode, offset, length uint64) error { + return syserror.EISDIR +} + +// PRead implements FileDescriptionImpl.PRead. +func (directoryFD) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts vfs.ReadOptions) (int64, error) { + return 0, syserror.EISDIR +} + +// Read implements FileDescriptionImpl.Read. +func (directoryFD) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.ReadOptions) (int64, error) { + return 0, syserror.EISDIR +} + +// PWrite implements FileDescriptionImpl.PWrite. +func (directoryFD) PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) { + return 0, syserror.EISDIR +} + +// Write implements FileDescriptionImpl.Write. +func (directoryFD) Write(ctx context.Context, src usermem.IOSequence, opts vfs.WriteOptions) (int64, error) { + return 0, syserror.EISDIR +} diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 8db337a2e..4dc8ef993 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -262,8 +262,17 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, vfsd *vfs.Dentr return nil, syserror.EOVERFLOW } - // FOPEN_KEEP_CACHE is the defualt flag for noOpen. - fd := fileDescription{OpenFlag: linux.FOPEN_KEEP_CACHE} + var fd *fileDescription + var fdImpl vfs.FileDescriptionImpl + if isDir { + directoryFD := &directoryFD{} + fd = &(directoryFD.fileDescription) + fdImpl = directoryFD + } else { + // FOPEN_KEEP_CACHE is the defualt flag for noOpen. + fd = &fileDescription{OpenFlag: linux.FOPEN_KEEP_CACHE} + fdImpl = fd + } // Only send open request when FUSE server support open or is opening a directory. if !i.fs.conn.noOpen || isDir { kernelTask := kernel.TaskFromContext(ctx) @@ -330,7 +339,7 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, vfsd *vfs.Dentr i.attributeTime = 0 } - if err := fd.vfsfd.Init(&fd, opts.Flags, rp.Mount(), vfsd, fdOptions); err != nil { + if err := fd.vfsfd.Init(fdImpl, opts.Flags, rp.Mount(), vfsd, fdOptions); err != nil { return nil, err } return &fd.vfsfd, nil @@ -374,6 +383,18 @@ func (i *inode) NewSymlink(ctx context.Context, name, target string) (*vfs.Dentr return i.newEntry(ctx, name, linux.S_IFLNK, linux.FUSE_SYMLINK, &in) } +// NewDir implements kernfs.Inode.NewDir. +func (i *inode) NewDir(ctx context.Context, name string, opts vfs.MkdirOptions) (*vfs.Dentry, error) { + in := linux.FUSEMkdirIn{ + MkdirMeta: linux.FUSEMkdirMeta{ + Mode: uint32(opts.Mode), + Umask: uint32(kernel.TaskFromContext(ctx).FSContext().Umask()), + }, + Name: name, + } + return i.newEntry(ctx, name, linux.S_IFDIR, linux.FUSE_MKDIR, &in) +} + // newEntry calls FUSE server for entry creation and allocates corresponding entry according to response. // Shared by FUSE_MKNOD, FUSE_MKDIR, FUSE_SYMLINK, FUSE_LINK and FUSE_LOOKUP. func (i *inode) newEntry(ctx context.Context, name string, fileType linux.FileMode, opcode linux.FUSEOpcode, payload marshal.Marshallable) (*vfs.Dentry, error) { diff --git a/test/fuse/BUILD b/test/fuse/BUILD index c2bdcf1ba..8bde81e3c 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -31,3 +31,8 @@ syscall_test( fuse = "True", test = "//test/fuse/linux:readlink_test", ) + +syscall_test( + fuse = "True", + test = "//test/fuse/linux:mkdir_test", +) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index d3e8ca148..298ea11f8 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -84,6 +84,20 @@ cc_binary( ], ) +cc_binary( + name = "mkdir_test", + testonly = 1, + srcs = ["mkdir_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:temp_umask", + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_library( name = "fuse_base", testonly = 1, diff --git a/test/fuse/linux/mkdir_test.cc b/test/fuse/linux/mkdir_test.cc new file mode 100644 index 000000000..9647cb93f --- /dev/null +++ b/test/fuse/linux/mkdir_test.cc @@ -0,0 +1,88 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/temp_umask.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class MkdirTest : public FuseTest { + protected: + const std::string test_dir_ = "test_dir"; + const mode_t perms_ = S_IRWXU | S_IRWXG | S_IRWXO; +}; + +TEST_F(MkdirTest, CreateDir) { + const std::string test_dir_path_ = + JoinPath(mount_point_.path().c_str(), test_dir_); + const mode_t new_umask = 0077; + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out), + }; + struct fuse_entry_out out_payload = DefaultEntryOut(S_IFDIR | perms_, 5); + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_MKDIR, iov_out); + TempUmask mask(new_umask); + ASSERT_THAT(mkdir(test_dir_path_.c_str(), 0777), SyscallSucceeds()); + + struct fuse_in_header in_header; + struct fuse_mkdir_in in_payload; + std::vector actual_dir(test_dir_.length() + 1); + auto iov_in = FuseGenerateIovecs(in_header, in_payload, actual_dir); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_header.len, + sizeof(in_header) + sizeof(in_payload) + test_dir_.length() + 1); + EXPECT_EQ(in_header.opcode, FUSE_MKDIR); + EXPECT_EQ(in_payload.mode & 0777, perms_ & ~new_umask); + EXPECT_EQ(in_payload.umask, new_umask); + EXPECT_EQ(std::string(actual_dir.data()), test_dir_); +} + +TEST_F(MkdirTest, FileTypeError) { + const std::string test_dir_path_ = + JoinPath(mount_point_.path().c_str(), test_dir_); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out), + }; + struct fuse_entry_out out_payload = DefaultEntryOut(S_IFREG | perms_, 5); + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_MKDIR, iov_out); + ASSERT_THAT(mkdir(test_dir_path_.c_str(), 0777), SyscallFailsWithErrno(EIO)); + SkipServerActualRequest(); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From 713400d6b0f2eef6a368bc9cb76a15ee3334d1e5 Mon Sep 17 00:00:00 2001 From: Jinmou Li Date: Wed, 15 Jul 2020 18:32:51 +0000 Subject: Implement FUSE_READ Fixes #3206 --- pkg/abi/linux/fuse.go | 37 ++++ pkg/sentry/fsimpl/fuse/BUILD | 3 + pkg/sentry/fsimpl/fuse/connection.go | 6 +- pkg/sentry/fsimpl/fuse/dev.go | 4 +- pkg/sentry/fsimpl/fuse/fusefs.go | 130 +++++++++-- pkg/sentry/fsimpl/fuse/init.go | 3 +- pkg/sentry/fsimpl/fuse/read_write.go | 152 +++++++++++++ pkg/sentry/fsimpl/fuse/regular_file.go | 125 +++++++++++ test/fuse/BUILD | 5 + test/fuse/linux/BUILD | 13 ++ test/fuse/linux/fuse_base.cc | 17 +- test/fuse/linux/fuse_base.h | 13 +- test/fuse/linux/read_test.cc | 390 +++++++++++++++++++++++++++++++++ 13 files changed, 862 insertions(+), 36 deletions(-) create mode 100644 pkg/sentry/fsimpl/fuse/read_write.go create mode 100644 pkg/sentry/fsimpl/fuse/regular_file.go create mode 100644 test/fuse/linux/read_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index 4ef0ab9a7..0ece7b756 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -184,6 +184,13 @@ const ( FUSE_KERNEL_MINOR_VERSION = 31 ) +// Constants relevant to FUSE operations. +const ( + FUSE_NAME_MAX = 1024 + FUSE_PAGE_SIZE = 4096 + FUSE_DIRENT_ALIGN = 8 +) + // FUSEInitIn is the request sent by the kernel to the daemon, // to negotiate the version and flags. // @@ -392,6 +399,36 @@ type FUSEOpenOut struct { // OpenFlag for the opened file. OpenFlag uint32 +} + +// FUSE_READ flags, consistent with the ones in include/uapi/linux/fuse.h. +const ( + FUSE_READ_LOCKOWNER = 1 << 1 +) + +// FUSEReadIn is the request sent by the kernel to the daemon +// for FUSE_READ. +// +// +marshal +type FUSEReadIn struct { + // Fh is the file handle in userspace. + Fh uint64 + + // Offset is the read offset. + Offset uint64 + + // Size is the number of bytes to read. + Size uint32 + + // ReadFlags for this FUSE_READ request. + // Currently only contains FUSE_READ_LOCKOWNER. + ReadFlags uint32 + + // LockOwner is the id of the lock owner if there is one. + LockOwner uint64 + + // Flags for the underlying file. + Flags uint32 _ uint32 } diff --git a/pkg/sentry/fsimpl/fuse/BUILD b/pkg/sentry/fsimpl/fuse/BUILD index b9dcee96a..241d50c3d 100644 --- a/pkg/sentry/fsimpl/fuse/BUILD +++ b/pkg/sentry/fsimpl/fuse/BUILD @@ -36,7 +36,9 @@ go_library( "fusefs.go", "init.go", "inode_refs.go", + "read_write.go", "register.go", + "regular_file.go", "request_list.go", ], visibility = ["//pkg/sentry:internal"], @@ -46,6 +48,7 @@ go_library( "//pkg/log", "//pkg/marshal", "//pkg/refs", + "//pkg/safemem", "//pkg/sentry/fsimpl/devtmpfs", "//pkg/sentry/fsimpl/kernfs", "//pkg/sentry/kernel", diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index 9cfd67158..236165652 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -161,6 +161,7 @@ type connection struct { bgLock sync.Mutex // maxRead is the maximum size of a read buffer in in bytes. + // Initialized from a fuse fs parameter. maxRead uint32 // maxWrite is the maximum size of a write buffer in bytes. @@ -206,7 +207,7 @@ type connection struct { } // newFUSEConnection creates a FUSE connection to fd. -func newFUSEConnection(_ context.Context, fd *vfs.FileDescription, maxInFlightRequests uint64) (*connection, error) { +func newFUSEConnection(_ context.Context, fd *vfs.FileDescription, opts *filesystemOptions) (*connection, error) { // Mark the device as ready so it can be used. /dev/fuse can only be used if the FD was used to // mount a FUSE filesystem. fuseFD := fd.Impl().(*DeviceFD) @@ -216,13 +217,14 @@ func newFUSEConnection(_ context.Context, fd *vfs.FileDescription, maxInFlightRe hdrLen := uint32((*linux.FUSEHeaderOut)(nil).SizeBytes()) fuseFD.writeBuf = make([]byte, hdrLen) fuseFD.completions = make(map[linux.FUSEOpID]*futureResponse) - fuseFD.fullQueueCh = make(chan struct{}, maxInFlightRequests) + fuseFD.fullQueueCh = make(chan struct{}, opts.maxActiveRequests) fuseFD.writeCursor = 0 return &connection{ fd: fuseFD, maxBackground: fuseDefaultMaxBackground, congestionThreshold: fuseDefaultCongestionThreshold, + maxRead: opts.maxRead, maxPages: fuseDefaultMaxPagesPerReq, initializedChan: make(chan struct{}), connected: true, diff --git a/pkg/sentry/fsimpl/fuse/dev.go b/pkg/sentry/fsimpl/fuse/dev.go index e2de8e097..fd3592e32 100644 --- a/pkg/sentry/fsimpl/fuse/dev.go +++ b/pkg/sentry/fsimpl/fuse/dev.go @@ -401,10 +401,12 @@ func (fd *DeviceFD) sendError(ctx context.Context, errno int32, req *Request) er // receiver is going to be waiting on the future channel. This is to be used by: // FUSE_INIT. func (fd *DeviceFD) noReceiverAction(ctx context.Context, r *Response) error { - if r.opcode == linux.FUSE_INIT { + switch r.opcode { + case linux.FUSE_INIT: creds := auth.CredentialsFromContext(ctx) rootUserNs := kernel.KernelFromContext(ctx).RootUserNamespace() return fd.fs.conn.InitRecv(r, creds.HasCapabilityIn(linux.CAP_SYS_ADMIN, rootUserNs)) + // TODO(gvisor.dev/issue/3247): support async read: correctly process the response using information from r.options. } return nil diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 4dc8ef993..65e22ba4d 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -16,6 +16,7 @@ package fuse import ( + "math" "strconv" "sync/atomic" @@ -58,6 +59,11 @@ type filesystemOptions struct { // exist at any time. Any further requests will block when trying to // Call the server. maxActiveRequests uint64 + + // maxRead is the max number of bytes to read, + // specified as "max_read" in fs parameters. + // If not specified by user, use math.MaxUint32 as default value. + maxRead uint32 } // filesystem implements vfs.FilesystemImpl. @@ -144,6 +150,21 @@ func (fsType FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt // Set the maxInFlightRequests option. fsopts.maxActiveRequests = maxActiveRequestsDefault + if maxReadStr, ok := mopts["max_read"]; ok { + delete(mopts, "max_read") + maxRead, err := strconv.ParseUint(maxReadStr, 10, 32) + if err != nil { + log.Warningf("%s.GetFilesystem: invalid max_read: max_read=%s", fsType.Name(), maxReadStr) + return nil, nil, syserror.EINVAL + } + if maxRead < fuseMinMaxRead { + maxRead = fuseMinMaxRead + } + fsopts.maxRead = uint32(maxRead) + } else { + fsopts.maxRead = math.MaxUint32 + } + // Check for unparsed options. if len(mopts) != 0 { log.Warningf("%s.GetFilesystem: unknown options: %v", fsType.Name(), mopts) @@ -179,7 +200,7 @@ func NewFUSEFilesystem(ctx context.Context, devMinor uint32, opts *filesystemOpt opts: opts, } - conn, err := newFUSEConnection(ctx, device, opts.maxActiveRequests) + conn, err := newFUSEConnection(ctx, device, opts) if err != nil { log.Warningf("fuse.NewFUSEFilesystem: NewFUSEConnection failed with error: %v", err) return nil, syserror.EINVAL @@ -244,6 +265,7 @@ func (fs *filesystem) newInode(nodeID uint64, attr linux.FUSEAttr) *kernfs.Dentr i := &inode{fs: fs, NodeID: nodeID} creds := auth.Credentials{EffectiveKGID: auth.KGID(attr.UID), EffectiveKUID: auth.KUID(attr.UID)} i.InodeAttrs.Init(&creds, linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.FileMode(attr.Mode)) + atomic.StoreUint64(&i.size, attr.Size) i.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) i.EnableLeakCheck() i.dentry.Init(i) @@ -269,10 +291,13 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, vfsd *vfs.Dentr fd = &(directoryFD.fileDescription) fdImpl = directoryFD } else { - // FOPEN_KEEP_CACHE is the defualt flag for noOpen. - fd = &fileDescription{OpenFlag: linux.FOPEN_KEEP_CACHE} - fdImpl = fd + regularFD := ®ularFileFD{} + fd = &(regularFD.fileDescription) + fdImpl = regularFD } + // FOPEN_KEEP_CACHE is the defualt flag for noOpen. + fd.OpenFlag = linux.FOPEN_KEEP_CACHE + // Only send open request when FUSE server support open or is opening a directory. if !i.fs.conn.noOpen || isDir { kernelTask := kernel.TaskFromContext(ctx) @@ -281,21 +306,25 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, vfsd *vfs.Dentr return nil, syserror.EINVAL } + // Build the request. var opcode linux.FUSEOpcode if isDir { opcode = linux.FUSE_OPENDIR } else { opcode = linux.FUSE_OPEN } + in := linux.FUSEOpenIn{Flags: opts.Flags & ^uint32(linux.O_CREAT|linux.O_EXCL|linux.O_NOCTTY)} if !i.fs.conn.atomicOTrunc { in.Flags &= ^uint32(linux.O_TRUNC) } + req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.NodeID, opcode, &in) if err != nil { return nil, err } + // Send the request and receive the reply. res, err := i.fs.conn.Call(kernelTask, req) if err != nil { return nil, err @@ -309,15 +338,17 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, vfsd *vfs.Dentr if err := res.UnmarshalPayload(&out); err != nil { return nil, err } + + // Process the reply. fd.OpenFlag = out.OpenFlag + if isDir { + fd.OpenFlag &= ^uint32(linux.FOPEN_DIRECT_IO) + } + fd.Fh = out.Fh } } - if isDir { - fd.OpenFlag &= ^uint32(linux.FOPEN_DIRECT_IO) - } - // TODO(gvisor.dev/issue/3234): invalidate mmap after implemented it for FUSE Inode fd.DirectIO = fd.OpenFlag&linux.FOPEN_DIRECT_IO != 0 fdOptions := &vfs.FileDescriptionOptions{} @@ -457,6 +488,16 @@ func (i *inode) Readlink(ctx context.Context, mnt *vfs.Mount) (string, error) { return i.link, nil } +// getFUSEAttr returns a linux.FUSEAttr of this inode stored in local cache. +// TODO(gvisor.dev/issue/3679): Add support for other fields. +func (i *inode) getFUSEAttr() linux.FUSEAttr { + return linux.FUSEAttr{ + Ino: i.Ino(), + Size: atomic.LoadUint64(&i.size), + Mode: uint32(i.Mode()), + } +} + // statFromFUSEAttr makes attributes from linux.FUSEAttr to linux.Statx. The // opts.Sync attribute is ignored since the synchronization is handled by the // FUSE server. @@ -510,47 +551,90 @@ func statFromFUSEAttr(attr linux.FUSEAttr, mask, devMinor uint32) linux.Statx { return stat } -// Stat implements kernfs.Inode.Stat. -func (i *inode) Stat(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOptions) (linux.Statx, error) { - fusefs := fs.Impl().(*filesystem) - conn := fusefs.conn - task, creds := kernel.TaskFromContext(ctx), auth.CredentialsFromContext(ctx) +// 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) { + attributeVersion := atomic.LoadUint64(&i.fs.conn.attributeVersion) + + // TODO(gvisor.dev/issue/3679): send the request only if + // - invalid local cache for fields specified in the opts.Mask + // - forced update + // - i.attributeTime expired + // If local cache is still valid, return local cache. + // Currently we always send a request, + // and we always set the metadata with the new result, + // unless attributeVersion has changed. + + task := kernel.TaskFromContext(ctx) if task == nil { log.Warningf("couldn't get kernel task from context") - return linux.Statx{}, syserror.EINVAL + return linux.FUSEAttr{}, syserror.EINVAL } + 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. - req, err := conn.NewRequest(creds, uint32(task.ThreadID()), i.NodeID, linux.FUSE_GETATTR, &in) + req, err := i.fs.conn.NewRequest(creds, uint32(task.ThreadID()), i.NodeID, linux.FUSE_GETATTR, &in) if err != nil { - return linux.Statx{}, err + return linux.FUSEAttr{}, err } - res, err := conn.Call(task, req) + res, err := i.fs.conn.Call(task, req) if err != nil { - return linux.Statx{}, err + return linux.FUSEAttr{}, err } if err := res.Error(); err != nil { - return linux.Statx{}, err + return linux.FUSEAttr{}, err } var out linux.FUSEGetAttrOut if err := res.UnmarshalPayload(&out); err != nil { - return linux.Statx{}, err + return linux.FUSEAttr{}, err } - // Set all metadata into kernfs.InodeAttrs. + // Local version is newer, return the local one. + // Skip the update. + if attributeVersion != 0 && atomic.LoadUint64(&i.attributeVersion) > attributeVersion { + return i.getFUSEAttr(), nil + } + + // Set the metadata of kernfs.InodeAttrs. if err := i.SetStat(ctx, fs, creds, vfs.SetStatOptions{ - Stat: statFromFUSEAttr(out.Attr, linux.STATX_ALL, fusefs.devMinor), + Stat: statFromFUSEAttr(out.Attr, linux.STATX_ALL, i.fs.devMinor), }); err != nil { + return linux.FUSEAttr{}, err + } + + // Set the size if no error (after SetStat() check). + atomic.StoreUint64(&i.size, out.Attr.Size) + + return out.Attr, nil +} + +// 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 { + // Never need atime for internal purposes. + _, err := i.getAttr(ctx, i.fs.VFSFilesystem(), vfs.StatOptions{ + Mask: linux.STATX_BASIC_STATS &^ linux.STATX_ATIME, + }) + 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) + if err != nil { return linux.Statx{}, err } - return statFromFUSEAttr(out.Attr, opts.Mask, fusefs.devMinor), nil + return statFromFUSEAttr(attr, opts.Mask, i.fs.devMinor), nil } // DecRef implements kernfs.Inode. diff --git a/pkg/sentry/fsimpl/fuse/init.go b/pkg/sentry/fsimpl/fuse/init.go index 779c2bd3f..2ff2542b6 100644 --- a/pkg/sentry/fsimpl/fuse/init.go +++ b/pkg/sentry/fsimpl/fuse/init.go @@ -29,9 +29,10 @@ const ( // Follow the same behavior as unix fuse implementation. fuseMaxTimeGranNs = 1000000000 - // Minimum value for MaxWrite. + // Minimum value for MaxWrite and MaxRead. // Follow the same behavior as unix fuse implementation. fuseMinMaxWrite = 4096 + fuseMinMaxRead = 4096 // Temporary default value for max readahead, 128kb. fuseDefaultMaxReadahead = 131072 diff --git a/pkg/sentry/fsimpl/fuse/read_write.go b/pkg/sentry/fsimpl/fuse/read_write.go new file mode 100644 index 000000000..4ef8531dc --- /dev/null +++ b/pkg/sentry/fsimpl/fuse/read_write.go @@ -0,0 +1,152 @@ +// 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. + +package fuse + +import ( + "io" + "sync/atomic" + + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/sentry/kernel" + "gvisor.dev/gvisor/pkg/sentry/kernel/auth" + "gvisor.dev/gvisor/pkg/syserror" + "gvisor.dev/gvisor/pkg/usermem" +) + +// ReadInPages sends FUSE_READ requests for the size after round it up to +// a multiple of page size, blocks on it for reply, processes the reply +// and returns the payload (or joined payloads) as a byte slice. +// This is used for the general purpose reading. +// We do not support direct IO (which read the exact number of bytes) +// at this moment. +func (fs *filesystem) ReadInPages(ctx context.Context, fd *regularFileFD, off uint64, size uint32) ([][]byte, uint32, error) { + attributeVersion := atomic.LoadUint64(&fs.conn.attributeVersion) + + t := kernel.TaskFromContext(ctx) + if t == nil { + log.Warningf("fusefs.Read: couldn't get kernel task from context") + return nil, 0, syserror.EINVAL + } + + // Round up to a multiple of page size. + readSize, _ := usermem.PageRoundUp(uint64(size)) + + // One request cannnot exceed either maxRead or maxPages. + maxPages := fs.conn.maxRead >> usermem.PageShift + if maxPages > uint32(fs.conn.maxPages) { + maxPages = uint32(fs.conn.maxPages) + } + + var outs [][]byte + var sizeRead uint32 + + // readSize is a multiple of usermem.PageSize. + // Always request bytes as a multiple of pages. + pagesRead, pagesToRead := uint32(0), uint32(readSize>>usermem.PageShift) + + // Reuse the same struct for unmarshalling to avoid unnecessary memory allocation. + in := linux.FUSEReadIn{ + Fh: fd.Fh, + LockOwner: 0, // TODO(gvisor.dev/issue/3245): file lock + ReadFlags: 0, // TODO(gvisor.dev/issue/3245): |= linux.FUSE_READ_LOCKOWNER + Flags: fd.statusFlags(), + } + + // This loop is intended for fragmented read where the bytes to read is + // larger than either the maxPages or maxRead. + // For the majority of reads with normal size, this loop should only + // execute once. + for pagesRead < pagesToRead { + pagesCanRead := pagesToRead - pagesRead + if pagesCanRead > maxPages { + pagesCanRead = maxPages + } + + in.Offset = off + (uint64(pagesRead) << usermem.PageShift) + in.Size = pagesCanRead << usermem.PageShift + + req, err := fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(t.ThreadID()), fd.inode().NodeID, linux.FUSE_READ, &in) + if err != nil { + return nil, 0, err + } + + // TODO(gvisor.dev/issue/3247): support async read. + + res, err := fs.conn.Call(t, req) + if err != nil { + return nil, 0, err + } + if err := res.Error(); err != nil { + return nil, 0, err + } + + // Not enough bytes in response, + // either we reached EOF, + // or the FUSE server sends back a response + // that cannot even fit the hdr. + if len(res.data) <= res.hdr.SizeBytes() { + // We treat both case as EOF here for now + // since there is no reliable way to detect + // the over-short hdr case. + break + } + + // Directly using the slice to avoid extra copy. + out := res.data[res.hdr.SizeBytes():] + + outs = append(outs, out) + sizeRead += uint32(len(out)) + + pagesRead += pagesCanRead + } + + defer fs.ReadCallback(ctx, fd, off, size, sizeRead, attributeVersion) + + // No bytes returned: offset >= EOF. + if len(outs) == 0 { + return nil, 0, io.EOF + } + + return outs, sizeRead, nil +} + +// ReadCallback updates several information after receiving a read response. +// Due to readahead, sizeRead can be larger than size. +func (fs *filesystem) ReadCallback(ctx context.Context, fd *regularFileFD, off uint64, size uint32, sizeRead uint32, attributeVersion uint64) { + // TODO(gvisor.dev/issue/3247): support async read. + // If this is called by an async read, correctly process it. + // May need to update the signature. + + i := fd.inode() + // TODO(gvisor.dev/issue/1193): Invalidate or update atime. + + // Reached EOF. + if sizeRead < size { + // TODO(gvisor.dev/issue/3630): If we have writeback cache, then we need to fill this hole. + // Might need to update the buf to be returned from the Read(). + + // Update existing size. + newSize := off + uint64(sizeRead) + fs.conn.mu.Lock() + if attributeVersion == i.attributeVersion && newSize < atomic.LoadUint64(&i.size) { + fs.conn.attributeVersion++ + i.attributeVersion = i.fs.conn.attributeVersion + atomic.StoreUint64(&i.size, newSize) + } + fs.conn.mu.Unlock() + } +} diff --git a/pkg/sentry/fsimpl/fuse/regular_file.go b/pkg/sentry/fsimpl/fuse/regular_file.go new file mode 100644 index 000000000..37ce4e268 --- /dev/null +++ b/pkg/sentry/fsimpl/fuse/regular_file.go @@ -0,0 +1,125 @@ +// 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. + +package fuse + +import ( + "io" + "math" + "sync" + "sync/atomic" + + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/sentry/vfs" + "gvisor.dev/gvisor/pkg/syserror" + "gvisor.dev/gvisor/pkg/usermem" +) + +type regularFileFD struct { + fileDescription + + // off is the file offset. + off int64 + // offMu protects off. + offMu sync.Mutex +} + +// PRead implements vfs.FileDescriptionImpl.PRead. +func (fd *regularFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts vfs.ReadOptions) (int64, error) { + if offset < 0 { + return 0, syserror.EINVAL + } + + // Check that flags are supported. + // + // TODO(gvisor.dev/issue/2601): Support select preadv2 flags. + if opts.Flags&^linux.RWF_HIPRI != 0 { + return 0, syserror.EOPNOTSUPP + } + + size := dst.NumBytes() + if size == 0 { + // Early return if count is 0. + return 0, nil + } else if size > math.MaxUint32 { + // FUSE only supports uint32 for size. + // Overflow. + return 0, syserror.EINVAL + } + + // TODO(gvisor.dev/issue/3678): Add direct IO support. + + inode := fd.inode() + + // Reading beyond EOF, update file size if outdated. + if uint64(offset+size) > atomic.LoadUint64(&inode.size) { + if err := inode.reviseAttr(ctx); err != nil { + return 0, err + } + // If the offset after update is still too large, return error. + if uint64(offset) >= atomic.LoadUint64(&inode.size) { + return 0, io.EOF + } + } + + // Truncate the read with updated file size. + fileSize := atomic.LoadUint64(&inode.size) + if uint64(offset+size) > fileSize { + size = int64(fileSize) - offset + } + + buffers, n, err := inode.fs.ReadInPages(ctx, fd, uint64(offset), uint32(size)) + if err != nil { + return 0, err + } + + // TODO(gvisor.dev/issue/3237): support indirect IO (e.g. caching), + // store the bytes that were read ahead. + + // Update the number of bytes to copy for short read. + if n < uint32(size) { + size = int64(n) + } + + // Copy the bytes read to the dst. + // This loop is intended for fragmented reads. + // For the majority of reads, this loop only execute once. + var copied int64 + for _, buffer := range buffers { + toCopy := int64(len(buffer)) + if copied+toCopy > size { + toCopy = size - copied + } + cp, err := dst.DropFirst64(copied).CopyOut(ctx, buffer[:toCopy]) + if err != nil { + return 0, err + } + if int64(cp) != toCopy { + return 0, syserror.EIO + } + copied += toCopy + } + + return copied, nil +} + +// Read implements vfs.FileDescriptionImpl.Read. +func (fd *regularFileFD) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.ReadOptions) (int64, error) { + fd.offMu.Lock() + n, err := fd.PRead(ctx, dst, fd.off, opts) + fd.off += n + fd.offMu.Unlock() + return n, err +} diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 8bde81e3c..cae51ce49 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -36,3 +36,8 @@ syscall_test( fuse = "True", test = "//test/fuse/linux:mkdir_test", ) + +syscall_test( + fuse = "True", + test = "//test/fuse/linux:read_test", +) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index 298ea11f8..8afd28f16 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -112,3 +112,16 @@ cc_library( "@com_google_absl//absl/strings:str_format", ], ) + +cc_binary( + name = "read_test", + testonly = 1, + srcs = ["read_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:test_main", + "//test/util:test_util", + ], +) \ No newline at end of file diff --git a/test/fuse/linux/fuse_base.cc b/test/fuse/linux/fuse_base.cc index 98b4e1466..e3c6b585c 100644 --- a/test/fuse/linux/fuse_base.cc +++ b/test/fuse/linux/fuse_base.cc @@ -129,7 +129,8 @@ void FuseTest::SkipServerActualRequest() { // Sends the `kSetInodeLookup` command, expected mode, and the path of the // inode to create under the mount point. -void FuseTest::SetServerInodeLookup(const std::string& path, mode_t mode) { +void FuseTest::SetServerInodeLookup(const std::string& path, mode_t mode, + uint64_t size) { uint32_t cmd = static_cast(FuseTestCmd::kSetInodeLookup); EXPECT_THAT(RetryEINTR(write)(sock_[0], &cmd, sizeof(cmd)), SyscallSucceedsWithValue(sizeof(cmd))); @@ -137,6 +138,9 @@ void FuseTest::SetServerInodeLookup(const std::string& path, mode_t mode) { EXPECT_THAT(RetryEINTR(write)(sock_[0], &mode, sizeof(mode)), SyscallSucceedsWithValue(sizeof(mode))); + EXPECT_THAT(RetryEINTR(write)(sock_[0], &size, sizeof(size)), + SyscallSucceedsWithValue(sizeof(size))); + // Pad 1 byte for null-terminate c-string. EXPECT_THAT(RetryEINTR(write)(sock_[0], path.c_str(), path.size() + 1), SyscallSucceedsWithValue(path.size() + 1)); @@ -144,10 +148,10 @@ void FuseTest::SetServerInodeLookup(const std::string& path, mode_t mode) { WaitServerComplete(); } -void FuseTest::MountFuse() { +void FuseTest::MountFuse(const char* mountOpts) { EXPECT_THAT(dev_fd_ = open("/dev/fuse", O_RDWR), SyscallSucceeds()); - std::string mount_opts = absl::StrFormat("fd=%d,%s", dev_fd_, kMountOpts); + std::string mount_opts = absl::StrFormat("fd=%d,%s", dev_fd_, mountOpts); mount_point_ = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); EXPECT_THAT(mount("fuse", mount_point_.path().c_str(), "fuse", MS_NODEV | MS_NOSUID, mount_opts.c_str()), @@ -311,11 +315,15 @@ void FuseTest::ServerHandleCommand() { // request with this specific path comes in. void FuseTest::ServerReceiveInodeLookup() { mode_t mode; + uint64_t size; std::vector buf(FUSE_MIN_READ_BUFFER); EXPECT_THAT(RetryEINTR(read)(sock_[1], &mode, sizeof(mode)), SyscallSucceedsWithValue(sizeof(mode))); + EXPECT_THAT(RetryEINTR(read)(sock_[1], &size, sizeof(size)), + SyscallSucceedsWithValue(sizeof(size))); + EXPECT_THAT(RetryEINTR(read)(sock_[1], buf.data(), buf.size()), SyscallSucceeds()); @@ -332,6 +340,9 @@ void FuseTest::ServerReceiveInodeLookup() { // comply with the unqiueness of different path. ++nodeid_; + // Set the size. + out_payload.attr.size = size; + memcpy(buf.data(), &out_header, sizeof(out_header)); memcpy(buf.data() + sizeof(out_header), &out_payload, sizeof(out_payload)); lookups_.AddMemBlock(FUSE_LOOKUP, buf.data(), out_len); diff --git a/test/fuse/linux/fuse_base.h b/test/fuse/linux/fuse_base.h index ff4c4499d..452748d6d 100644 --- a/test/fuse/linux/fuse_base.h +++ b/test/fuse/linux/fuse_base.h @@ -137,7 +137,8 @@ class FuseTest : public ::testing::Test { // path, pretending there is an inode and avoid ENOENT when testing. If mode // is not given, it creates a regular file with mode 0600. void SetServerInodeLookup(const std::string& path, - mode_t mode = S_IFREG | S_IRUSR | S_IWUSR); + mode_t mode = S_IFREG | S_IRUSR | S_IWUSR, + uint64_t size = 512); // Called by the testing thread to ask the FUSE server for its next received // FUSE request. Be sure to use the corresponding struct of iovec to receive @@ -166,16 +167,16 @@ class FuseTest : public ::testing::Test { protected: TempPath mount_point_; - // Unmounts the mountpoint of the FUSE server. - void UnmountFuse(); - - private: // Opens /dev/fuse and inherit the file descriptor for the FUSE server. - void MountFuse(); + void MountFuse(const char* mountOpts = kMountOpts); // Creates a socketpair for communication and forks FUSE server. void SetUpFuseServer(); + // Unmounts the mountpoint of the FUSE server. + void UnmountFuse(); + + private: // Sends a FuseTestCmd and gets a uint32_t data from the FUSE server. inline uint32_t GetServerData(uint32_t cmd); diff --git a/test/fuse/linux/read_test.cc b/test/fuse/linux/read_test.cc new file mode 100644 index 000000000..c702651bd --- /dev/null +++ b/test/fuse/linux/read_test.cc @@ -0,0 +1,390 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class ReadTest : public FuseTest { + void SetUp() override { + FuseTest::SetUp(); + test_file_path_ = JoinPath(mount_point_.path().c_str(), test_file_); + } + + // TearDown overrides the parent's function + // to skip checking the unconsumed release request at the end. + void TearDown() override { UnmountFuse(); } + + protected: + const std::string test_file_ = "test_file"; + const mode_t test_file_mode_ = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO; + const uint64_t test_fh_ = 1; + const uint32_t open_flag_ = O_RDWR; + + std::string test_file_path_; + + PosixErrorOr OpenTestFile(const std::string &path, + uint64_t size = 512) { + SetServerInodeLookup(test_file_, test_file_mode_, size); + + struct fuse_out_header out_header_open = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_open_out), + }; + struct fuse_open_out out_payload_open = { + .fh = test_fh_, + .open_flags = open_flag_, + }; + auto iov_out_open = FuseGenerateIovecs(out_header_open, out_payload_open); + SetServerResponse(FUSE_OPEN, iov_out_open); + + auto res = Open(path.c_str(), open_flag_); + if (res.ok()) { + SkipServerActualRequest(); + } + return res; + } +}; + +class ReadTestSmallMaxRead : public ReadTest { + void SetUp() override { + MountFuse(mountOpts); + SetUpFuseServer(); + test_file_path_ = JoinPath(mount_point_.path().c_str(), test_file_); + } + + protected: + constexpr static char mountOpts[] = + "rootmode=755,user_id=0,group_id=0,max_read=4096"; + // 4096 is hard-coded as the max_read in mount options. + const int size_fragment = 4096; +}; + +TEST_F(ReadTest, ReadWhole) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Prepare for the read. + const int n_read = 5; + std::vector data(n_read); + RandomizeBuffer(data.data(), data.size()); + struct fuse_out_header out_header_read = { + .len = + static_cast(sizeof(struct fuse_out_header) + data.size()), + }; + auto iov_out_read = FuseGenerateIovecs(out_header_read, data); + SetServerResponse(FUSE_READ, iov_out_read); + + // Read the whole "file". + std::vector buf(n_read); + EXPECT_THAT(read(fd.get(), buf.data(), n_read), + SyscallSucceedsWithValue(n_read)); + + // Check the read request. + struct fuse_in_header in_header_read; + struct fuse_read_in in_payload_read; + auto iov_in = FuseGenerateIovecs(in_header_read, in_payload_read); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_header_read.len, + sizeof(in_header_read) + sizeof(in_payload_read)); + EXPECT_EQ(in_header_read.opcode, FUSE_READ); + EXPECT_EQ(in_payload_read.offset, 0); + EXPECT_EQ(buf, data); +} + +TEST_F(ReadTest, ReadPartial) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Prepare for the read. + const int n_data = 10; + std::vector data(n_data); + RandomizeBuffer(data.data(), data.size()); + // Note: due to read ahead, current read implementation will treat any + // response that is longer than requested as correct (i.e. not reach the EOF). + // Therefore, the test below should make sure the size to read does not exceed + // n_data. + struct fuse_out_header out_header_read = { + .len = + static_cast(sizeof(struct fuse_out_header) + data.size()), + }; + auto iov_out_read = FuseGenerateIovecs(out_header_read, data); + struct fuse_in_header in_header_read; + struct fuse_read_in in_payload_read; + auto iov_in = FuseGenerateIovecs(in_header_read, in_payload_read); + + std::vector buf(n_data); + + // Read 1 bytes. + SetServerResponse(FUSE_READ, iov_out_read); + EXPECT_THAT(read(fd.get(), buf.data(), 1), SyscallSucceedsWithValue(1)); + + // Check the 1-byte read request. + GetServerActualRequest(iov_in); + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_header_read.len, + sizeof(in_header_read) + sizeof(in_payload_read)); + EXPECT_EQ(in_header_read.opcode, FUSE_READ); + EXPECT_EQ(in_payload_read.offset, 0); + + // Read 3 bytes. + SetServerResponse(FUSE_READ, iov_out_read); + EXPECT_THAT(read(fd.get(), buf.data(), 3), SyscallSucceedsWithValue(3)); + + // Check the 3-byte read request. + GetServerActualRequest(iov_in); + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_payload_read.offset, 1); + + // Read 5 bytes. + SetServerResponse(FUSE_READ, iov_out_read); + EXPECT_THAT(read(fd.get(), buf.data(), 5), SyscallSucceedsWithValue(5)); + + // Check the 5-byte read request. + GetServerActualRequest(iov_in); + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_payload_read.offset, 4); +} + +TEST_F(ReadTest, PRead) { + const int file_size = 512; + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_, file_size)); + + // Prepare for the read. + const int n_read = 5; + std::vector data(n_read); + RandomizeBuffer(data.data(), data.size()); + struct fuse_out_header out_header_read = { + .len = + static_cast(sizeof(struct fuse_out_header) + data.size()), + }; + auto iov_out_read = FuseGenerateIovecs(out_header_read, data); + SetServerResponse(FUSE_READ, iov_out_read); + + // Read some bytes. + std::vector buf(n_read); + const int offset_read = file_size >> 1; + EXPECT_THAT(pread(fd.get(), buf.data(), n_read, offset_read), + SyscallSucceedsWithValue(n_read)); + + // Check the read request. + struct fuse_in_header in_header_read; + struct fuse_read_in in_payload_read; + auto iov_in = FuseGenerateIovecs(in_header_read, in_payload_read); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_header_read.len, + sizeof(in_header_read) + sizeof(in_payload_read)); + EXPECT_EQ(in_header_read.opcode, FUSE_READ); + EXPECT_EQ(in_payload_read.offset, offset_read); + EXPECT_EQ(buf, data); +} + +TEST_F(ReadTest, ReadZero) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Issue the read. + std::vector buf; + EXPECT_THAT(read(fd.get(), buf.data(), 0), SyscallSucceedsWithValue(0)); +} + +TEST_F(ReadTest, ReadShort) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Prepare for the short read. + const int n_read = 5; + std::vector data(n_read >> 1); + RandomizeBuffer(data.data(), data.size()); + struct fuse_out_header out_header_read = { + .len = + static_cast(sizeof(struct fuse_out_header) + data.size()), + }; + auto iov_out_read = FuseGenerateIovecs(out_header_read, data); + SetServerResponse(FUSE_READ, iov_out_read); + + // Read the whole "file". + std::vector buf(n_read); + EXPECT_THAT(read(fd.get(), buf.data(), n_read), + SyscallSucceedsWithValue(data.size())); + + // Check the read request. + struct fuse_in_header in_header_read; + struct fuse_read_in in_payload_read; + auto iov_in = FuseGenerateIovecs(in_header_read, in_payload_read); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_header_read.len, + sizeof(in_header_read) + sizeof(in_payload_read)); + EXPECT_EQ(in_header_read.opcode, FUSE_READ); + EXPECT_EQ(in_payload_read.offset, 0); + std::vector short_buf(buf.begin(), buf.begin() + data.size()); + EXPECT_EQ(short_buf, data); +} + +TEST_F(ReadTest, ReadShortEOF) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Prepare for the short read. + struct fuse_out_header out_header_read = { + .len = static_cast(sizeof(struct fuse_out_header)), + }; + auto iov_out_read = FuseGenerateIovecs(out_header_read); + SetServerResponse(FUSE_READ, iov_out_read); + + // Read the whole "file". + const int n_read = 10; + std::vector buf(n_read); + EXPECT_THAT(read(fd.get(), buf.data(), n_read), SyscallSucceedsWithValue(0)); + + // Check the read request. + struct fuse_in_header in_header_read; + struct fuse_read_in in_payload_read; + auto iov_in = FuseGenerateIovecs(in_header_read, in_payload_read); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_header_read.len, + sizeof(in_header_read) + sizeof(in_payload_read)); + EXPECT_EQ(in_header_read.opcode, FUSE_READ); + EXPECT_EQ(in_payload_read.offset, 0); +} + +TEST_F(ReadTestSmallMaxRead, ReadSmallMaxRead) { + const int n_fragment = 10; + const int n_read = size_fragment * n_fragment; + + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_, n_read)); + + // Prepare for the read. + std::vector data(size_fragment); + RandomizeBuffer(data.data(), data.size()); + struct fuse_out_header out_header_read = { + .len = + static_cast(sizeof(struct fuse_out_header) + data.size()), + }; + auto iov_out_read = FuseGenerateIovecs(out_header_read, data); + + for (int i = 0; i < n_fragment; ++i) { + SetServerResponse(FUSE_READ, iov_out_read); + } + + // Read the whole "file". + std::vector buf(n_read); + EXPECT_THAT(read(fd.get(), buf.data(), n_read), + SyscallSucceedsWithValue(n_read)); + + ASSERT_EQ(GetServerNumUnsentResponses(), 0); + ASSERT_EQ(GetServerNumUnconsumedRequests(), n_fragment); + + // Check each read segment. + struct fuse_in_header in_header_read; + struct fuse_read_in in_payload_read; + auto iov_in = FuseGenerateIovecs(in_header_read, in_payload_read); + + for (int i = 0; i < n_fragment; ++i) { + GetServerActualRequest(iov_in); + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_header_read.len, + sizeof(in_header_read) + sizeof(in_payload_read)); + EXPECT_EQ(in_header_read.opcode, FUSE_READ); + EXPECT_EQ(in_payload_read.offset, i * size_fragment); + EXPECT_EQ(in_payload_read.size, size_fragment); + + auto it = buf.begin() + i * size_fragment; + EXPECT_EQ(std::vector(it, it + size_fragment), data); + } +} + +TEST_F(ReadTestSmallMaxRead, ReadSmallMaxReadShort) { + const int n_fragment = 10; + const int n_read = size_fragment * n_fragment; + + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_, n_read)); + + // Prepare for the read. + std::vector data(size_fragment); + RandomizeBuffer(data.data(), data.size()); + struct fuse_out_header out_header_read = { + .len = + static_cast(sizeof(struct fuse_out_header) + data.size()), + }; + auto iov_out_read = FuseGenerateIovecs(out_header_read, data); + + for (int i = 0; i < n_fragment - 1; ++i) { + SetServerResponse(FUSE_READ, iov_out_read); + } + + // The last fragment is a short read. + std::vector half_data(data.begin(), data.begin() + (data.size() >> 1)); + struct fuse_out_header out_header_read_short = { + .len = static_cast(sizeof(struct fuse_out_header) + + half_data.size()), + }; + auto iov_out_read_short = + FuseGenerateIovecs(out_header_read_short, half_data); + SetServerResponse(FUSE_READ, iov_out_read_short); + + // Read the whole "file". + std::vector buf(n_read); + EXPECT_THAT(read(fd.get(), buf.data(), n_read), + SyscallSucceedsWithValue(n_read - (data.size() >> 1))); + + ASSERT_EQ(GetServerNumUnsentResponses(), 0); + ASSERT_EQ(GetServerNumUnconsumedRequests(), n_fragment); + + // Check each read segment. + struct fuse_in_header in_header_read; + struct fuse_read_in in_payload_read; + auto iov_in = FuseGenerateIovecs(in_header_read, in_payload_read); + + for (int i = 0; i < n_fragment; ++i) { + GetServerActualRequest(iov_in); + EXPECT_EQ(in_payload_read.fh, test_fh_); + EXPECT_EQ(in_header_read.len, + sizeof(in_header_read) + sizeof(in_payload_read)); + EXPECT_EQ(in_header_read.opcode, FUSE_READ); + EXPECT_EQ(in_payload_read.offset, i * size_fragment); + EXPECT_EQ(in_payload_read.size, size_fragment); + + auto it = buf.begin() + i * size_fragment; + if (i != n_fragment - 1) { + EXPECT_EQ(std::vector(it, it + data.size()), data); + } else { + EXPECT_EQ(std::vector(it, it + half_data.size()), half_data); + } + } +} + +} // namespace + +} // namespace testing +} // namespace gvisor \ No newline at end of file -- cgit v1.2.3 From bc07df88878f47a496f8b364b286bf0b11c0e76f Mon Sep 17 00:00:00 2001 From: Ridwan Sharif Date: Tue, 11 Aug 2020 12:13:01 -0400 Subject: Implement FUSE_RMDIR Fixes #3587 Co-authored-by: Craig Chi --- pkg/abi/linux/fuse.go | 26 ++++++++++++ pkg/sentry/fsimpl/fuse/fusefs.go | 27 +++++++++++++ pkg/sentry/fsimpl/kernfs/filesystem.go | 7 +++- test/fuse/BUILD | 6 +++ test/fuse/linux/BUILD | 14 +++++++ test/fuse/linux/rmdir_test.cc | 73 ++++++++++++++++++++++++++++++++++ 6 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 test/fuse/linux/rmdir_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index 0ece7b756..c75debb8c 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -569,3 +569,29 @@ func (r *FUSEMkdirIn) MarshalUnsafe(buf []byte) { func (r *FUSEMkdirIn) SizeBytes() int { return r.MkdirMeta.SizeBytes() + len(r.Name) + 1 } + +// FUSERmDirIn is the request sent by the kernel to the daemon +// when trying to remove a directory. +// +// Dynamically-sized objects cannot be marshalled. +type FUSERmDirIn struct { + marshal.StubMarshallable + + // Name is a directory name to be looked up. + Name string +} + +// MarshalUnsafe serializes r.name to the dst buffer. +func (r *FUSERmDirIn) MarshalUnsafe(buf []byte) { + copy(buf, r.Name) +} + +// SizeBytes is the size of the memory representation of FUSERmDirIn. +func (r *FUSERmDirIn) SizeBytes() int { + return len(r.Name) + 1 +} + +// UnmarshalUnsafe deserializes r.name from the src buffer. +func (r *FUSERmDirIn) UnmarshalUnsafe(src []byte) { + r.Name = string(src) +} diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 65e22ba4d..c55ea927a 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -426,6 +426,33 @@ func (i *inode) NewDir(ctx context.Context, name string, opts vfs.MkdirOptions) return i.newEntry(ctx, name, linux.S_IFDIR, linux.FUSE_MKDIR, &in) } +// RmDir implements kernfs.Inode.RmDir. +func (i *inode) RmDir(ctx context.Context, name string, child *vfs.Dentry) error { + fusefs := i.fs + task, creds := kernel.TaskFromContext(ctx), auth.CredentialsFromContext(ctx) + + in := linux.FUSERmDirIn{Name: name} + req, err := fusefs.conn.NewRequest(creds, uint32(task.ThreadID()), i.NodeID, linux.FUSE_RMDIR, &in) + if err != nil { + return err + } + + res, err := i.fs.conn.Call(task, req) + if err != nil { + return err + } + if err := res.Error(); err != nil { + return err + } + + // TODO(Before merging): When creating new nodes, should we add nodes to the ordered children? + // If so we'll probably need to call this. We will also need to add them with the writable flag when + // appropriate. + // return i.OrderedChildren.RmDir(ctx, name, child) + + return nil +} + // newEntry calls FUSE server for entry creation and allocates corresponding entry according to response. // Shared by FUSE_MKNOD, FUSE_MKDIR, FUSE_SYMLINK, FUSE_LINK and FUSE_LOOKUP. func (i *inode) newEntry(ctx context.Context, name string, fileType linux.FileMode, opcode linux.FUSEOpcode, payload marshal.Marshallable) (*vfs.Dentry, error) { diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index 7aaf1146d..2823c3b1a 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/pkg/sentry/fsimpl/kernfs/filesystem.go @@ -657,6 +657,10 @@ func (fs *Filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa func (fs *Filesystem) RmdirAt(ctx context.Context, rp *vfs.ResolvingPath) error { fs.mu.Lock() defer fs.mu.Unlock() + + // Store the name before walkExistingLocked as rp will be advanced past the + // name in the following call. + name := rp.Component() vfsd, inode, err := fs.walkExistingLocked(ctx, rp) fs.processDeferredDecRefsLocked(ctx) if err != nil { @@ -686,7 +690,8 @@ func (fs *Filesystem) RmdirAt(ctx context.Context, rp *vfs.ResolvingPath) error if err := virtfs.PrepareDeleteDentry(mntns, vfsd); err != nil { return err } - if err := parentDentry.inode.RmDir(ctx, rp.Component(), vfsd); err != nil { + + if err := parentDentry.inode.RmDir(ctx, name, vfsd); err != nil { virtfs.AbortDeleteDentry(vfsd) return err } diff --git a/test/fuse/BUILD b/test/fuse/BUILD index cae51ce49..30d2a871f 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -41,3 +41,9 @@ syscall_test( fuse = "True", test = "//test/fuse/linux:read_test", ) + +syscall_test( + test = "//test/fuse/linux:rmdir_test", + vfs2 = "True", + fuse = "True", +) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index 8afd28f16..159428fce 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -98,6 +98,20 @@ cc_binary( ], ) +cc_binary( + name = "rmdir_test", + testonly = 1, + srcs = ["rmdir_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, diff --git a/test/fuse/linux/rmdir_test.cc b/test/fuse/linux/rmdir_test.cc new file mode 100644 index 000000000..913d3f910 --- /dev/null +++ b/test/fuse/linux/rmdir_test.cc @@ -0,0 +1,73 @@ +// 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 +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fs_util.h" +#include "test/util/fuse_util.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class RmDirTest : public FuseTest { + protected: + const std::string test_dir_name_ = "test_dir"; + const mode_t test_dir_mode_ = S_IFDIR | S_IRWXU | S_IRWXG | S_IRWXO; +}; + +TEST_F(RmDirTest, NormalRmDir) { + const std::string test_dir_path_ = + JoinPath(mount_point_.path().c_str(), test_dir_name_); + + SetServerInodeLookup(test_dir_name_, test_dir_mode_); + + // RmDir code. + struct fuse_out_header rmdir_header = { + .len = sizeof(struct fuse_out_header), + }; + + auto iov_out = FuseGenerateIovecs(rmdir_header); + SetServerResponse(FUSE_RMDIR, iov_out); + + ASSERT_THAT(rmdir(test_dir_path_.c_str()), SyscallSucceeds()); + + struct fuse_in_header in_header; + std::vector actual_dirname(test_dir_name_.length() + 1); + auto iov_in = FuseGenerateIovecs(in_header, actual_dirname); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_header.len, sizeof(in_header) + test_dir_name_.length() + 1); + EXPECT_EQ(in_header.opcode, FUSE_RMDIR); + EXPECT_EQ(std::string(actual_dirname.data()), test_dir_name_); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From 4a5857d644ae0e62090bbbed86852dceca79395c Mon Sep 17 00:00:00 2001 From: Ridwan Sharif Date: Mon, 27 Jul 2020 14:42:31 -0400 Subject: 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 --- pkg/abi/linux/fuse.go | 131 ++++++++++++++++++- pkg/sentry/fsimpl/fuse/connection.go | 8 ++ pkg/sentry/fsimpl/fuse/directory.go | 54 ++++++++ pkg/sentry/fsimpl/fuse/file.go | 6 +- pkg/sentry/fsimpl/kernfs/kernfs.go | 2 +- pkg/sentry/vfs/file_description_impl_util.go | 2 +- test/fuse/BUILD | 6 +- test/fuse/linux/BUILD | 14 ++ test/fuse/linux/readdir_test.cc | 188 +++++++++++++++++++++++++++ 9 files changed, 403 insertions(+), 8 deletions(-) create mode 100644 test/fuse/linux/readdir_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index c75debb8c..e7b5f45de 100644 --- a/pkg/abi/linux/fuse.go +++ b/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) +} diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index 236165652..133306158 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/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 } diff --git a/pkg/sentry/fsimpl/fuse/directory.go b/pkg/sentry/fsimpl/fuse/directory.go index 44d41712a..8c59680e8 100644 --- a/pkg/sentry/fsimpl/fuse/directory.go +++ b/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 +} diff --git a/pkg/sentry/fsimpl/fuse/file.go b/pkg/sentry/fsimpl/fuse/file.go index 01d20caf6..186ec2362 100644 --- a/pkg/sentry/fsimpl/fuse/file.go +++ b/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) } diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index f656e2a8b..61189af25 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/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, diff --git a/pkg/sentry/vfs/file_description_impl_util.go b/pkg/sentry/vfs/file_description_impl_util.go index 68b80a951..2b668fd89 100644 --- a/pkg/sentry/vfs/file_description_impl_util.go +++ b/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 diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 30d2a871f..a1b29aa33 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -43,7 +43,11 @@ syscall_test( ) syscall_test( + fuse = "True", test = "//test/fuse/linux:rmdir_test", - vfs2 = "True", +) + +syscall_test( fuse = "True", + test = "//test/fuse/linux:readdir_test", ) diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index 159428fce..23c9fba31 100644 --- a/test/fuse/linux/BUILD +++ b/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, diff --git a/test/fuse/linux/readdir_test.cc b/test/fuse/linux/readdir_test.cc new file mode 100644 index 000000000..17fb630ee --- /dev/null +++ b/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 +#include +#include +#include +#include +#include +#include + +#include + +#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(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 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 \ No newline at end of file -- cgit v1.2.3 From d51ddcefdc46a1007f7345bfa2f7006bb820b157 Mon Sep 17 00:00:00 2001 From: Ridwan Sharif Date: Wed, 19 Aug 2020 20:11:59 -0400 Subject: fuse: use safe go_marshal API for FUSE Until #3698 is resolved, this change is needed to ensure we're not corrupting memory anywhere. --- pkg/abi/linux/fuse.go | 97 +++++++++++++++++++++++++++++++++++- pkg/marshal/marshal_impl_util.go | 2 +- pkg/sentry/fsimpl/fuse/connection.go | 9 ++-- 3 files changed, 103 insertions(+), 5 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index e7b5f45de..d105c5176 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -359,7 +359,12 @@ type FUSELookupIn struct { // MarshalUnsafe serializes r.name to the dst buffer. func (r *FUSELookupIn) MarshalUnsafe(buf []byte) { - copy(buf, []byte(r.Name)) + copy(buf, r.Name) +} + +// MarshalBytes serializes r.name to the dst buffer. +func (r *FUSELookupIn) MarshalBytes(buf []byte) { + copy(buf, r.Name) } // SizeBytes is the size of the memory representation of FUSELookupIn. @@ -491,6 +496,12 @@ func (r *FUSEMknodIn) MarshalUnsafe(buf []byte) { copy(buf[r.MknodMeta.SizeBytes():], r.Name) } +// MarshalBytes serializes r.MknodMeta and r.Name to the dst buffer. +func (r *FUSEMknodIn) MarshalBytes(buf []byte) { + r.MknodMeta.MarshalBytes(buf[:r.MknodMeta.SizeBytes()]) + copy(buf[r.MknodMeta.SizeBytes():], r.Name) +} + // SizeBytes is the size of the memory representation of FUSEMknodIn. // 1 extra byte for null-terminated string. func (r *FUSEMknodIn) SizeBytes() int { @@ -518,6 +529,13 @@ func (r *FUSESymLinkIn) MarshalUnsafe(buf []byte) { copy(buf[len(r.Name)+1:], r.Target) } +// MarshalBytes serializes r.Name and r.Target to the dst buffer. +// Left null-termination at end of r.Name and r.Target. +func (r *FUSESymLinkIn) MarshalBytes(buf []byte) { + copy(buf, r.Name) + copy(buf[len(r.Name)+1:], r.Target) +} + // SizeBytes is the size of the memory representation of FUSESymLinkIn. // 2 extra bytes for null-terminated string. func (r *FUSESymLinkIn) SizeBytes() int { @@ -530,6 +548,9 @@ type FUSEEmptyIn struct{ marshal.StubMarshallable } // MarshalUnsafe do nothing for marshal. func (r *FUSEEmptyIn) MarshalUnsafe(buf []byte) {} +// MarshalBytes do nothing for marshal. +func (r *FUSEEmptyIn) MarshalBytes(buf []byte) {} + // SizeBytes is 0 for empty request. func (r *FUSEEmptyIn) SizeBytes() int { return 0 @@ -567,6 +588,12 @@ func (r *FUSEMkdirIn) MarshalUnsafe(buf []byte) { copy(buf[r.MkdirMeta.SizeBytes():], r.Name) } +// MarshalBytes serializes r.MkdirMeta and r.Name to the dst buffer. +func (r *FUSEMkdirIn) MarshalBytes(buf []byte) { + r.MkdirMeta.MarshalBytes(buf[:r.MkdirMeta.SizeBytes()]) + copy(buf[r.MkdirMeta.SizeBytes():], r.Name) +} + // SizeBytes is the size of the memory representation of FUSEMkdirIn. // 1 extra byte for null-terminated Name string. func (r *FUSEMkdirIn) SizeBytes() int { @@ -589,6 +616,11 @@ func (r *FUSERmDirIn) MarshalUnsafe(buf []byte) { copy(buf, r.Name) } +// MarshalBytes serializes r.name to the dst buffer. +func (r *FUSERmDirIn) MarshalBytes(buf []byte) { + copy(buf, r.Name) +} + // SizeBytes is the size of the memory representation of FUSERmDirIn. func (r *FUSERmDirIn) SizeBytes() int { return len(r.Name) + 1 @@ -599,6 +631,11 @@ func (r *FUSERmDirIn) UnmarshalUnsafe(src []byte) { r.Name = string(src) } +// UnmarshalBytes deserializes r.name from the src buffer. +func (r *FUSERmDirIn) UnmarshalBytes(src []byte) { + r.Name = string(src) +} + // FUSEDirents is a list of Dirents received from the FUSE daemon server. // It is used for FUSE_READDIR. // @@ -649,6 +686,14 @@ func (r *FUSEDirents) MarshalUnsafe(dst []byte) { } } +// MarshalBytes serializes FUSEDirents to the dst buffer. +func (r *FUSEDirents) MarshalBytes(dst []byte) { + for _, dirent := range r.Dirents { + dirent.MarshalBytes(dst) + dst = dst[dirent.SizeBytes():] + } +} + // SizeBytes is the size of the memory representation of FUSEDirents. func (r *FUSEDirents) SizeBytes() int { var sizeBytes int @@ -683,6 +728,30 @@ func (r *FUSEDirents) UnmarshalUnsafe(src []byte) { } } +// UnmarshalBytes deserializes FUSEDirents from the src buffer. +func (r *FUSEDirents) UnmarshalBytes(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.UnmarshalBytes(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) @@ -692,6 +761,15 @@ func (r *FUSEDirent) MarshalUnsafe(dst []byte) { name.MarshalUnsafe(dst) } +// MarshalBytes serializes FUSEDirent to the dst buffer. +func (r *FUSEDirent) MarshalBytes(dst []byte) { + r.Meta.MarshalBytes(dst) + dst = dst[r.Meta.SizeBytes():] + + name := primitive.ByteSlice(r.Name) + name.MarshalBytes(dst) +} + // SizeBytes is the size of the memory representation of FUSEDirent. func (r *FUSEDirent) SizeBytes() int { dataSize := r.Meta.SizeBytes() + len(r.Name) @@ -718,3 +796,20 @@ func (r *FUSEDirent) UnmarshalUnsafe(src []byte) { name.UnmarshalUnsafe(src[:r.Meta.NameLen]) r.Name = string(name) } + +// UnmarshalBytes deserializes FUSEDirent from the src buffer. +func (r *FUSEDirent) UnmarshalBytes(src []byte) { + r.Meta.UnmarshalBytes(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.UnmarshalBytes(src[:r.Meta.NameLen]) + r.Name = string(name) +} diff --git a/pkg/marshal/marshal_impl_util.go b/pkg/marshal/marshal_impl_util.go index 69f33321e..ea75e09f2 100644 --- a/pkg/marshal/marshal_impl_util.go +++ b/pkg/marshal/marshal_impl_util.go @@ -44,7 +44,7 @@ func (StubMarshallable) MarshalBytes(dst []byte) { // UnmarshalBytes implements Marshallable.UnmarshalBytes. func (StubMarshallable) UnmarshalBytes(src []byte) { - panic("Please implement your own UnMarshalBytes function") + panic("Please implement your own UnmarshalBytes function") } // Packed implements Marshallable.Packed. diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index 133306158..0e70d37ec 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -271,8 +271,10 @@ func (conn *connection) NewRequest(creds *auth.Credentials, pid uint32, ino uint } buf := make([]byte, hdr.Len) - hdr.MarshalUnsafe(buf[:hdrLen]) - payload.MarshalUnsafe(buf[hdrLen:]) + + // TODO(gVisor.dev/3698): Use the unsafe version once go_marshal is safe to use again. + hdr.MarshalBytes(buf[:hdrLen]) + payload.MarshalBytes(buf[hdrLen:]) return &Request{ id: hdr.Unique, @@ -360,7 +362,8 @@ func (r *Response) UnmarshalPayload(m marshal.Marshallable) error { return nil } - m.UnmarshalUnsafe(r.data[hdrLen:]) + // TODO(gVisor.dev/3698): Use the unsafe version once go_marshal is safe to use again. + m.UnmarshalBytes(r.data[hdrLen:]) return nil } -- cgit v1.2.3 From 449986264f9277c4c6174fc82294fc6644923e8b Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Thu, 20 Aug 2020 12:31:52 -0700 Subject: Support multiple FUSE kernel versions of FUSE_INIT response struct The fuse_init_out struct changes in different FUSE kernel versions. A FUSE server may implement older versions of fuse_init_out, but they share common attributes from the beginning. Implement variable-length marshallable interface to support older versions of ABI. Fixes #3707 --- pkg/abi/linux/fuse.go | 58 ++++++++++++++++++++++++++++++++++++ pkg/sentry/fsimpl/fuse/connection.go | 5 ++++ pkg/sentry/fsimpl/fuse/init.go | 6 ++-- 3 files changed, 66 insertions(+), 3 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index d105c5176..ca4ee5e80 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -15,6 +15,7 @@ package linux import ( + "gvisor.dev/gvisor/pkg/usermem" "gvisor.dev/gvisor/tools/go_marshal/marshal" "gvisor.dev/gvisor/tools/go_marshal/primitive" ) @@ -262,6 +263,63 @@ type FUSEInitOut struct { _ [8]uint32 } +// FUSEInitRes is a variable-length wrapper of FUSEInitOut. The FUSE server may +// implement older version of FUSE protocol, which contains a FUSEInitOut with +// less attributes. +// +// Dynamically-sized objects cannot be marshalled. +type FUSEInitRes struct { + marshal.StubMarshallable + + // InitOut contains the response from the FUSE server. + InitOut FUSEInitOut + + // Len is the total length of bytes of the response. + Len uint32 +} + +// UnMarshalBytes deserializes src to the InitOut attribute in a FUSEInitRes. +func (r *FUSEInitRes) UnmarshalBytes(src []byte) { + out := &r.InitOut + + // Introduced before FUSE kernel version 7.13. + out.Major = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + out.Minor = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + out.MaxReadahead = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + out.Flags = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + out.MaxBackground = uint16(usermem.ByteOrder.Uint16(src[:2])) + src = src[2:] + out.CongestionThreshold = uint16(usermem.ByteOrder.Uint16(src[:2])) + src = src[2:] + out.MaxWrite = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + + // Introduced in FUSE kernel version 7.23. + if len(src) >= 4 { + out.TimeGran = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + } + // Introduced in FUSE kernel version 7.28. + if len(src) >= 2 { + out.MaxPages = uint16(usermem.ByteOrder.Uint16(src[:2])) + src = src[2:] + } + // Introduced in FUSE kernel version 7.31. + if len(src) >= 2 { + out.MapAlignment = uint16(usermem.ByteOrder.Uint16(src[:2])) + src = src[2:] + } +} + +// SizeBytes is the size of the payload of the FUSE_INIT response. +func (r *FUSEInitRes) SizeBytes() int { + return int(r.Len) +} + // 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/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index 0e70d37ec..dc169b497 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -346,6 +346,11 @@ func (r *Response) Error() error { return error(sysErrNo) } +// DataLen returns the size of the response without the header. +func (r *Response) DataLen() uint32 { + return r.hdr.Len - uint32(r.hdr.SizeBytes()) +} + // UnmarshalPayload unmarshals the response data into m. func (r *Response) UnmarshalPayload(m marshal.Marshallable) error { hdrLen := r.hdr.SizeBytes() diff --git a/pkg/sentry/fsimpl/fuse/init.go b/pkg/sentry/fsimpl/fuse/init.go index 2ff2542b6..6384cbbdb 100644 --- a/pkg/sentry/fsimpl/fuse/init.go +++ b/pkg/sentry/fsimpl/fuse/init.go @@ -76,12 +76,12 @@ func (conn *connection) InitRecv(res *Response, hasSysAdminCap bool) error { return err } - var out linux.FUSEInitOut - if err := res.UnmarshalPayload(&out); err != nil { + initRes := linux.FUSEInitRes{Len: res.DataLen()} + if err := res.UnmarshalPayload(&initRes); err != nil { return err } - return conn.initProcessReply(&out, hasSysAdminCap) + return conn.initProcessReply(&initRes.InitOut, hasSysAdminCap) } // Process the FUSE_INIT reply from the FUSE server. -- cgit v1.2.3 From 1d8029022e8ca45e177e36405506d42874f125e9 Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Wed, 2 Sep 2020 13:30:56 -0700 Subject: fuse: remove unused marshalling functions This commit removes unused marshalling functions in linux abi package and moves self-defined FUSEInitRes wrapper to fuse package. Updates #3707 --- pkg/abi/linux/fuse.go | 175 ----------------------------- pkg/sentry/fsimpl/fuse/BUILD | 1 + pkg/sentry/fsimpl/fuse/init.go | 4 +- pkg/sentry/fsimpl/fuse/request_response.go | 78 +++++++++++++ 4 files changed, 81 insertions(+), 177 deletions(-) create mode 100644 pkg/sentry/fsimpl/fuse/request_response.go (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index ca4ee5e80..5616290a5 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -15,7 +15,6 @@ package linux import ( - "gvisor.dev/gvisor/pkg/usermem" "gvisor.dev/gvisor/tools/go_marshal/marshal" "gvisor.dev/gvisor/tools/go_marshal/primitive" ) @@ -263,63 +262,6 @@ type FUSEInitOut struct { _ [8]uint32 } -// FUSEInitRes is a variable-length wrapper of FUSEInitOut. The FUSE server may -// implement older version of FUSE protocol, which contains a FUSEInitOut with -// less attributes. -// -// Dynamically-sized objects cannot be marshalled. -type FUSEInitRes struct { - marshal.StubMarshallable - - // InitOut contains the response from the FUSE server. - InitOut FUSEInitOut - - // Len is the total length of bytes of the response. - Len uint32 -} - -// UnMarshalBytes deserializes src to the InitOut attribute in a FUSEInitRes. -func (r *FUSEInitRes) UnmarshalBytes(src []byte) { - out := &r.InitOut - - // Introduced before FUSE kernel version 7.13. - out.Major = uint32(usermem.ByteOrder.Uint32(src[:4])) - src = src[4:] - out.Minor = uint32(usermem.ByteOrder.Uint32(src[:4])) - src = src[4:] - out.MaxReadahead = uint32(usermem.ByteOrder.Uint32(src[:4])) - src = src[4:] - out.Flags = uint32(usermem.ByteOrder.Uint32(src[:4])) - src = src[4:] - out.MaxBackground = uint16(usermem.ByteOrder.Uint16(src[:2])) - src = src[2:] - out.CongestionThreshold = uint16(usermem.ByteOrder.Uint16(src[:2])) - src = src[2:] - out.MaxWrite = uint32(usermem.ByteOrder.Uint32(src[:4])) - src = src[4:] - - // Introduced in FUSE kernel version 7.23. - if len(src) >= 4 { - out.TimeGran = uint32(usermem.ByteOrder.Uint32(src[:4])) - src = src[4:] - } - // Introduced in FUSE kernel version 7.28. - if len(src) >= 2 { - out.MaxPages = uint16(usermem.ByteOrder.Uint16(src[:2])) - src = src[2:] - } - // Introduced in FUSE kernel version 7.31. - if len(src) >= 2 { - out.MapAlignment = uint16(usermem.ByteOrder.Uint16(src[:2])) - src = src[2:] - } -} - -// SizeBytes is the size of the payload of the FUSE_INIT response. -func (r *FUSEInitRes) SizeBytes() int { - return int(r.Len) -} - // FUSEGetAttrIn is the request sent by the kernel to the daemon, // to get the attribute of a inode. // @@ -415,11 +357,6 @@ type FUSELookupIn struct { Name string } -// MarshalUnsafe serializes r.name to the dst buffer. -func (r *FUSELookupIn) MarshalUnsafe(buf []byte) { - copy(buf, r.Name) -} - // MarshalBytes serializes r.name to the dst buffer. func (r *FUSELookupIn) MarshalBytes(buf []byte) { copy(buf, r.Name) @@ -548,12 +485,6 @@ type FUSEMknodIn struct { Name string } -// MarshalUnsafe serializes r.MknodMeta and r.Name to the dst buffer. -func (r *FUSEMknodIn) MarshalUnsafe(buf []byte) { - r.MknodMeta.MarshalUnsafe(buf[:r.MknodMeta.SizeBytes()]) - copy(buf[r.MknodMeta.SizeBytes():], r.Name) -} - // MarshalBytes serializes r.MknodMeta and r.Name to the dst buffer. func (r *FUSEMknodIn) MarshalBytes(buf []byte) { r.MknodMeta.MarshalBytes(buf[:r.MknodMeta.SizeBytes()]) @@ -580,13 +511,6 @@ type FUSESymLinkIn struct { Target string } -// MarshalUnsafe serializes r.Name and r.Target to the dst buffer. -// Left null-termination at end of r.Name and r.Target. -func (r *FUSESymLinkIn) MarshalUnsafe(buf []byte) { - copy(buf, r.Name) - copy(buf[len(r.Name)+1:], r.Target) -} - // MarshalBytes serializes r.Name and r.Target to the dst buffer. // Left null-termination at end of r.Name and r.Target. func (r *FUSESymLinkIn) MarshalBytes(buf []byte) { @@ -603,9 +527,6 @@ func (r *FUSESymLinkIn) SizeBytes() int { // FUSEEmptyIn is used by operations without request body. type FUSEEmptyIn struct{ marshal.StubMarshallable } -// MarshalUnsafe do nothing for marshal. -func (r *FUSEEmptyIn) MarshalUnsafe(buf []byte) {} - // MarshalBytes do nothing for marshal. func (r *FUSEEmptyIn) MarshalBytes(buf []byte) {} @@ -640,12 +561,6 @@ type FUSEMkdirIn struct { Name string } -// MarshalUnsafe serializes r.MkdirMeta and r.Name to the dst buffer. -func (r *FUSEMkdirIn) MarshalUnsafe(buf []byte) { - r.MkdirMeta.MarshalUnsafe(buf[:r.MkdirMeta.SizeBytes()]) - copy(buf[r.MkdirMeta.SizeBytes():], r.Name) -} - // MarshalBytes serializes r.MkdirMeta and r.Name to the dst buffer. func (r *FUSEMkdirIn) MarshalBytes(buf []byte) { r.MkdirMeta.MarshalBytes(buf[:r.MkdirMeta.SizeBytes()]) @@ -669,11 +584,6 @@ type FUSERmDirIn struct { Name string } -// MarshalUnsafe serializes r.name to the dst buffer. -func (r *FUSERmDirIn) MarshalUnsafe(buf []byte) { - copy(buf, r.Name) -} - // MarshalBytes serializes r.name to the dst buffer. func (r *FUSERmDirIn) MarshalBytes(buf []byte) { copy(buf, r.Name) @@ -684,16 +594,6 @@ func (r *FUSERmDirIn) SizeBytes() int { return len(r.Name) + 1 } -// UnmarshalUnsafe deserializes r.name from the src buffer. -func (r *FUSERmDirIn) UnmarshalUnsafe(src []byte) { - r.Name = string(src) -} - -// UnmarshalBytes deserializes r.name from the src buffer. -func (r *FUSERmDirIn) UnmarshalBytes(src []byte) { - r.Name = string(src) -} - // FUSEDirents is a list of Dirents received from the FUSE daemon server. // It is used for FUSE_READDIR. // @@ -736,22 +636,6 @@ type FUSEDirentMeta struct { 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():] - } -} - -// MarshalBytes serializes FUSEDirents to the dst buffer. -func (r *FUSEDirents) MarshalBytes(dst []byte) { - for _, dirent := range r.Dirents { - dirent.MarshalBytes(dst) - dst = dst[dirent.SizeBytes():] - } -} - // SizeBytes is the size of the memory representation of FUSEDirents. func (r *FUSEDirents) SizeBytes() int { var sizeBytes int @@ -762,30 +646,6 @@ func (r *FUSEDirents) SizeBytes() int { 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():] - } -} - // UnmarshalBytes deserializes FUSEDirents from the src buffer. func (r *FUSEDirents) UnmarshalBytes(src []byte) { for { @@ -810,24 +670,6 @@ func (r *FUSEDirents) UnmarshalBytes(src []byte) { } } -// 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) -} - -// MarshalBytes serializes FUSEDirent to the dst buffer. -func (r *FUSEDirent) MarshalBytes(dst []byte) { - r.Meta.MarshalBytes(dst) - dst = dst[r.Meta.SizeBytes():] - - name := primitive.ByteSlice(r.Name) - name.MarshalBytes(dst) -} - // SizeBytes is the size of the memory representation of FUSEDirent. func (r *FUSEDirent) SizeBytes() int { dataSize := r.Meta.SizeBytes() + len(r.Name) @@ -838,23 +680,6 @@ func (r *FUSEDirent) SizeBytes() int { 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) -} - // UnmarshalBytes deserializes FUSEDirent from the src buffer. func (r *FUSEDirent) UnmarshalBytes(src []byte) { r.Meta.UnmarshalBytes(src) diff --git a/pkg/sentry/fsimpl/fuse/BUILD b/pkg/sentry/fsimpl/fuse/BUILD index 241d50c3d..5085d0521 100644 --- a/pkg/sentry/fsimpl/fuse/BUILD +++ b/pkg/sentry/fsimpl/fuse/BUILD @@ -40,6 +40,7 @@ go_library( "register.go", "regular_file.go", "request_list.go", + "request_response.go", ], visibility = ["//pkg/sentry:internal"], deps = [ diff --git a/pkg/sentry/fsimpl/fuse/init.go b/pkg/sentry/fsimpl/fuse/init.go index 6384cbbdb..256b6fb65 100644 --- a/pkg/sentry/fsimpl/fuse/init.go +++ b/pkg/sentry/fsimpl/fuse/init.go @@ -76,12 +76,12 @@ func (conn *connection) InitRecv(res *Response, hasSysAdminCap bool) error { return err } - initRes := linux.FUSEInitRes{Len: res.DataLen()} + initRes := fuseInitRes{initLen: res.DataLen()} if err := res.UnmarshalPayload(&initRes); err != nil { return err } - return conn.initProcessReply(&initRes.InitOut, hasSysAdminCap) + return conn.initProcessReply(&initRes.initOut, hasSysAdminCap) } // Process the FUSE_INIT reply from the FUSE server. diff --git a/pkg/sentry/fsimpl/fuse/request_response.go b/pkg/sentry/fsimpl/fuse/request_response.go new file mode 100644 index 000000000..ae71b5e28 --- /dev/null +++ b/pkg/sentry/fsimpl/fuse/request_response.go @@ -0,0 +1,78 @@ +// 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. + +package fuse + +import ( + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/usermem" + "gvisor.dev/gvisor/tools/go_marshal/marshal" +) + +// fuseInitRes is a variable-length wrapper of linux.FUSEInitOut. The FUSE +// server may implement an older version of FUSE protocol, which contains a +// linux.FUSEInitOut with less attributes. +// +// Dynamically-sized objects cannot be marshalled. +type fuseInitRes struct { + marshal.StubMarshallable + + // initOut contains the response from the FUSE server. + initOut linux.FUSEInitOut + + // initLen is the total length of bytes of the response. + initLen uint32 +} + +// UnmarshalBytes deserializes src to the initOut attribute in a fuseInitRes. +func (r *fuseInitRes) UnmarshalBytes(src []byte) { + out := &r.initOut + + // Introduced before FUSE kernel version 7.13. + out.Major = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + out.Minor = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + out.MaxReadahead = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + out.Flags = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + out.MaxBackground = uint16(usermem.ByteOrder.Uint16(src[:2])) + src = src[2:] + out.CongestionThreshold = uint16(usermem.ByteOrder.Uint16(src[:2])) + src = src[2:] + out.MaxWrite = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + + // Introduced in FUSE kernel version 7.23. + if len(src) >= 4 { + out.TimeGran = uint32(usermem.ByteOrder.Uint32(src[:4])) + src = src[4:] + } + // Introduced in FUSE kernel version 7.28. + if len(src) >= 2 { + out.MaxPages = uint16(usermem.ByteOrder.Uint16(src[:2])) + src = src[2:] + } + // Introduced in FUSE kernel version 7.31. + if len(src) >= 2 { + out.MapAlignment = uint16(usermem.ByteOrder.Uint16(src[:2])) + src = src[2:] + } +} + +// SizeBytes is the size of the payload of the FUSE_INIT response. +func (r *fuseInitRes) SizeBytes() int { + return int(r.initLen) +} -- cgit v1.2.3 From e91c026672b9022d5554e20202648d322da0fe1d Mon Sep 17 00:00:00 2001 From: Jinmou Li Date: Wed, 2 Sep 2020 13:50:31 -0700 Subject: Downgrade FUSE minor version support and clarify comments --- pkg/abi/linux/fuse.go | 53 +++++++++++++----------------- pkg/sentry/fsimpl/fuse/connection.go | 20 +++++------ pkg/sentry/fsimpl/fuse/init.go | 2 -- pkg/sentry/fsimpl/fuse/request_response.go | 5 --- 4 files changed, 31 insertions(+), 49 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index 5616290a5..ed40df564 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -151,34 +151,27 @@ type FUSEWriteIn struct { } // FUSE_INIT flags, consistent with the ones in include/uapi/linux/fuse.h. +// Our taget version is 7.23 but we have few implemented in advance. const ( - FUSE_ASYNC_READ = 1 << 0 - FUSE_POSIX_LOCKS = 1 << 1 - FUSE_FILE_OPS = 1 << 2 - FUSE_ATOMIC_O_TRUNC = 1 << 3 - FUSE_EXPORT_SUPPORT = 1 << 4 - FUSE_BIG_WRITES = 1 << 5 - FUSE_DONT_MASK = 1 << 6 - FUSE_SPLICE_WRITE = 1 << 7 - FUSE_SPLICE_MOVE = 1 << 8 - FUSE_SPLICE_READ = 1 << 9 - FUSE_FLOCK_LOCKS = 1 << 10 - FUSE_HAS_IOCTL_DIR = 1 << 11 - FUSE_AUTO_INVAL_DATA = 1 << 12 - FUSE_DO_READDIRPLUS = 1 << 13 - FUSE_READDIRPLUS_AUTO = 1 << 14 - FUSE_ASYNC_DIO = 1 << 15 - FUSE_WRITEBACK_CACHE = 1 << 16 - FUSE_NO_OPEN_SUPPORT = 1 << 17 - FUSE_PARALLEL_DIROPS = 1 << 18 - FUSE_HANDLE_KILLPRIV = 1 << 19 - FUSE_POSIX_ACL = 1 << 20 - FUSE_ABORT_ERROR = 1 << 21 - FUSE_MAX_PAGES = 1 << 22 - FUSE_CACHE_SYMLINKS = 1 << 23 - FUSE_NO_OPENDIR_SUPPORT = 1 << 24 - FUSE_EXPLICIT_INVAL_DATA = 1 << 25 - FUSE_MAP_ALIGNMENT = 1 << 26 + FUSE_ASYNC_READ = 1 << 0 + FUSE_POSIX_LOCKS = 1 << 1 + FUSE_FILE_OPS = 1 << 2 + FUSE_ATOMIC_O_TRUNC = 1 << 3 + FUSE_EXPORT_SUPPORT = 1 << 4 + FUSE_BIG_WRITES = 1 << 5 + FUSE_DONT_MASK = 1 << 6 + FUSE_SPLICE_WRITE = 1 << 7 + FUSE_SPLICE_MOVE = 1 << 8 + FUSE_SPLICE_READ = 1 << 9 + FUSE_FLOCK_LOCKS = 1 << 10 + FUSE_HAS_IOCTL_DIR = 1 << 11 + FUSE_AUTO_INVAL_DATA = 1 << 12 + FUSE_DO_READDIRPLUS = 1 << 13 + FUSE_READDIRPLUS_AUTO = 1 << 14 + FUSE_ASYNC_DIO = 1 << 15 + FUSE_WRITEBACK_CACHE = 1 << 16 + FUSE_NO_OPEN_SUPPORT = 1 << 17 + FUSE_MAX_PAGES = 1 << 22 // From FUSE 7.28 ) // currently supported FUSE protocol version numbers. @@ -214,7 +207,7 @@ type FUSEInitIn struct { } // FUSEInitOut is the reply sent by the daemon to the kernel -// for FUSEInitIn. +// for FUSEInitIn. We target FUSE 7.23; this struct supports 7.28. // // +marshal type FUSEInitOut struct { @@ -255,9 +248,7 @@ type FUSEInitOut struct { // if the value from daemon is too large. MaxPages uint16 - // MapAlignment is an unknown field and not used by this package at this moment. - // Use as a placeholder to be consistent with the FUSE protocol. - MapAlignment uint16 + _ uint16 _ [8]uint32 } diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index dc169b497..5f20483b7 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -86,16 +86,22 @@ type connection struct { // attributeVersion is the version of connection's attributes. attributeVersion uint64 + // We target FUSE 7.23. // The following FUSE_INIT flags are currently unsupported by this implementation: // - FUSE_EXPORT_SUPPORT - // - FUSE_HANDLE_KILLPRIV // - FUSE_POSIX_LOCKS: requires POSIX locks // - FUSE_FLOCK_LOCKS: requires POSIX locks // - FUSE_AUTO_INVAL_DATA: requires page caching eviction - // - FUSE_EXPLICIT_INVAL_DATA: requires page caching eviction // - FUSE_DO_READDIRPLUS/FUSE_READDIRPLUS_AUTO: requires FUSE_READDIRPLUS implementation // - FUSE_ASYNC_DIO - // - FUSE_POSIX_ACL: affects defaultPermissions, posixACL, xattr handler + // - FUSE_PARALLEL_DIROPS (7.25) + // - FUSE_HANDLE_KILLPRIV (7.26) + // - FUSE_POSIX_ACL: affects defaultPermissions, posixACL, xattr handler (7.26) + // - FUSE_ABORT_ERROR (7.27) + // - FUSE_CACHE_SYMLINKS (7.28) + // - FUSE_NO_OPENDIR_SUPPORT (7.29) + // - FUSE_EXPLICIT_INVAL_DATA: requires page caching eviction (7.30) + // - FUSE_MAP_ALIGNMENT (7.31) // initialized after receiving FUSE_INIT reply. // Until it's set, suspend sending FUSE requests. @@ -182,19 +188,11 @@ type connection struct { // Negotiated and only set in INIT. asyncRead bool - // abortErr is true if kernel need to return an unique read error after abort. - // Negotiated and only set in INIT. - abortErr bool - // writebackCache is true for write-back cache policy, // false for write-through policy. // Negotiated and only set in INIT. writebackCache bool - // cacheSymlinks if filesystem needs to cache READLINK responses in page cache. - // Negotiated and only set in INIT. - cacheSymlinks bool - // bigWrites if doing multi-page cached writes. // Negotiated and only set in INIT. bigWrites bool diff --git a/pkg/sentry/fsimpl/fuse/init.go b/pkg/sentry/fsimpl/fuse/init.go index 256b6fb65..a47309b6e 100644 --- a/pkg/sentry/fsimpl/fuse/init.go +++ b/pkg/sentry/fsimpl/fuse/init.go @@ -132,8 +132,6 @@ func (conn *connection) initProcessReply(out *linux.FUSEInitOut, hasSysAdminCap conn.bigWrites = out.Flags&linux.FUSE_BIG_WRITES != 0 conn.dontMask = out.Flags&linux.FUSE_DONT_MASK != 0 conn.writebackCache = out.Flags&linux.FUSE_WRITEBACK_CACHE != 0 - conn.cacheSymlinks = out.Flags&linux.FUSE_CACHE_SYMLINKS != 0 - conn.abortErr = out.Flags&linux.FUSE_ABORT_ERROR != 0 // TODO(gvisor.dev/issue/3195): figure out how to use TimeGran (0 < TimeGran <= fuseMaxTimeGranNs). diff --git a/pkg/sentry/fsimpl/fuse/request_response.go b/pkg/sentry/fsimpl/fuse/request_response.go index ae71b5e28..a69b21221 100644 --- a/pkg/sentry/fsimpl/fuse/request_response.go +++ b/pkg/sentry/fsimpl/fuse/request_response.go @@ -65,11 +65,6 @@ func (r *fuseInitRes) UnmarshalBytes(src []byte) { out.MaxPages = uint16(usermem.ByteOrder.Uint16(src[:2])) src = src[2:] } - // Introduced in FUSE kernel version 7.31. - if len(src) >= 2 { - out.MapAlignment = uint16(usermem.ByteOrder.Uint16(src[:2])) - src = src[2:] - } } // SizeBytes is the size of the payload of the FUSE_INIT response. -- cgit v1.2.3 From 18f1e1c91b05059c333197a2a6198716c12508e7 Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Thu, 3 Sep 2020 14:06:59 -0700 Subject: Implement FUSE_CREATE FUSE_CREATE is called when issuing creat(2) or open(2) with O_CREAT. It creates a new file on the FUSE filesystem. Fixes #3825 --- pkg/abi/linux/fuse.go | 42 +++++++++++++ pkg/sentry/fsimpl/fuse/fusefs.go | 18 ++++++ test/fuse/BUILD | 4 ++ test/fuse/linux/BUILD | 17 +++++- test/fuse/linux/create_test.cc | 128 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 test/fuse/linux/create_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index ed40df564..adc04dbcc 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -445,6 +445,48 @@ type FUSEReleaseIn struct { LockOwner uint64 } +// FUSECreateMeta contains all the static fields of FUSECreateIn, +// which is used for FUSE_CREATE. +// +// +marshal +type FUSECreateMeta struct { + // Flags of the creating file. + Flags uint32 + + // Mode is the mode of the creating file. + Mode uint32 + + // Umask is the current file mode creation mask. + Umask uint32 + _ uint32 +} + +// FUSECreateIn contains all the arguments sent by the kernel to the daemon, to +// atomically create and open a new regular file. +// +// Dynamically-sized objects cannot be marshalled. +type FUSECreateIn struct { + marshal.StubMarshallable + + // CreateMeta contains mode, rdev and umash field for FUSE_MKNODS. + CreateMeta FUSECreateMeta + + // Name is the name of the node to create. + Name string +} + +// MarshalBytes serializes r.CreateMeta and r.Name to the dst buffer. +func (r *FUSECreateIn) MarshalBytes(buf []byte) { + r.CreateMeta.MarshalBytes(buf[:r.CreateMeta.SizeBytes()]) + copy(buf[r.CreateMeta.SizeBytes():], r.Name) +} + +// SizeBytes is the size of the memory representation of FUSECreateIn. +// 1 extra byte for null-terminated string. +func (r *FUSECreateIn) SizeBytes() int { + return r.CreateMeta.SizeBytes() + len(r.Name) + 1 +} + // FUSEMknodMeta contains all the static fields of FUSEMknodIn, // which is used for FUSE_MKNOD. // diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index e1bbb4b52..cfae9ed0d 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -392,6 +392,24 @@ func (inode) Valid(ctx context.Context) bool { return true } +// NewFile implements kernfs.Inode.NewFile. +func (i *inode) NewFile(ctx context.Context, name string, opts vfs.OpenOptions) (*vfs.Dentry, error) { + kernelTask := kernel.TaskFromContext(ctx) + if kernelTask == nil { + log.Warningf("fusefs.Inode.NewFile: couldn't get kernel task from context", i.NodeID) + return nil, syserror.EINVAL + } + in := linux.FUSECreateIn{ + CreateMeta: linux.FUSECreateMeta{ + Flags: opts.Flags, + Mode: uint32(opts.Mode) | linux.S_IFREG, + Umask: uint32(kernelTask.FSContext().Umask()), + }, + Name: name, + } + return i.newEntry(ctx, name, linux.S_IFREG, linux.FUSE_CREATE, &in) +} + // NewNode implements kernfs.Inode.NewNode. func (i *inode) NewNode(ctx context.Context, name string, opts vfs.MknodOptions) (*vfs.Dentry, error) { in := linux.FUSEMknodIn{ diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 02498b3a1..1a6b5b516 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -52,6 +52,10 @@ syscall_test( test = "//test/fuse/linux:readdir_test", ) +syscall_test( + fuse = "True", + test = "//test/fuse/linux:create_test", +) syscall_test( size = "large", diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index 23c9fba31..2bb956af9 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -152,4 +152,19 @@ cc_binary( "//test/util:test_main", "//test/util:test_util", ], -) \ No newline at end of file +) + +cc_binary( + name = "create_test", + testonly = 1, + srcs = ["create_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fs_util", + "//test/util:fuse_util", + "//test/util:temp_umask", + "//test/util:test_main", + "//test/util:test_util", + ], +) diff --git a/test/fuse/linux/create_test.cc b/test/fuse/linux/create_test.cc new file mode 100644 index 000000000..9a0219a58 --- /dev/null +++ b/test/fuse/linux/create_test.cc @@ -0,0 +1,128 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fs_util.h" +#include "test/util/fuse_util.h" +#include "test/util/temp_umask.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class CreateTest : public FuseTest { + protected: + const std::string test_file_name_ = "test_file"; + const mode_t mode = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO; +}; + +TEST_F(CreateTest, CreateFile) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_name_); + + // Ensure the file doesn't exist. + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header), + .error = -ENOENT, + }; + auto iov_out = FuseGenerateIovecs(out_header); + SetServerResponse(FUSE_LOOKUP, iov_out); + + // creat(2) is equal to open(2) with open_flags O_CREAT | O_WRONLY | O_TRUNC. + const mode_t new_mask = S_IWGRP | S_IWOTH; + const int open_flags = O_CREAT | O_WRONLY | O_TRUNC; + out_header.error = 0; + out_header.len = sizeof(struct fuse_out_header) + + sizeof(struct fuse_entry_out) + sizeof(struct fuse_open_out); + struct fuse_entry_out entry_payload = DefaultEntryOut(mode & ~new_mask, 2); + struct fuse_open_out out_payload = { + .fh = 1, + .open_flags = open_flags, + }; + iov_out = FuseGenerateIovecs(out_header, entry_payload, out_payload); + SetServerResponse(FUSE_CREATE, iov_out); + + // kernfs generates a successive FUSE_OPEN after the file is created. Linux's + // fuse kernel module will not send this FUSE_OPEN after creat(2). + out_header.len = + sizeof(struct fuse_out_header) + sizeof(struct fuse_open_out); + iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_OPEN, iov_out); + + int fd; + TempUmask mask(new_mask); + EXPECT_THAT(fd = creat(test_file_path.c_str(), mode), SyscallSucceeds()); + EXPECT_THAT(fcntl(fd, F_GETFL), + SyscallSucceedsWithValue(open_flags & O_ACCMODE)); + + struct fuse_in_header in_header; + struct fuse_create_in in_payload; + std::vector name(test_file_name_.size() + 1); + auto iov_in = FuseGenerateIovecs(in_header, in_payload, name); + + // Skip the request of FUSE_LOOKUP. + SkipServerActualRequest(); + + // Get the first FUSE_CREATE. + GetServerActualRequest(iov_in); + EXPECT_EQ(in_header.len, sizeof(in_header) + sizeof(in_payload) + + test_file_name_.size() + 1); + EXPECT_EQ(in_header.opcode, FUSE_CREATE); + EXPECT_EQ(in_payload.flags, open_flags); + EXPECT_EQ(in_payload.mode, mode & ~new_mask); + EXPECT_EQ(in_payload.umask, new_mask); + EXPECT_EQ(std::string(name.data()), test_file_name_); + + // Get the successive FUSE_OPEN. + struct fuse_open_in in_payload_open; + iov_in = FuseGenerateIovecs(in_header, in_payload_open); + GetServerActualRequest(iov_in); + EXPECT_EQ(in_header.len, sizeof(in_header) + sizeof(in_payload_open)); + EXPECT_EQ(in_header.opcode, FUSE_OPEN); + EXPECT_EQ(in_payload_open.flags, open_flags & O_ACCMODE); + + EXPECT_THAT(close(fd), SyscallSucceeds()); + // Skip the FUSE_RELEASE. + SkipServerActualRequest(); +} + +TEST_F(CreateTest, CreateFileAlreadyExists) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_name_); + + const int open_flags = O_CREAT | O_EXCL; + + SetServerInodeLookup(test_file_name_); + + EXPECT_THAT(open(test_file_path.c_str(), mode, open_flags), + SyscallFailsWithErrno(EEXIST)); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From 98faed55e682cf34bb713c37b063a7d1da5e8352 Mon Sep 17 00:00:00 2001 From: Jinmou Li Date: Tue, 1 Sep 2020 01:49:57 +0000 Subject: Implement FUSE_WRITE This commit adds basic write(2) support for FUSE. --- pkg/abi/linux/fuse.go | 67 +++++--- pkg/sentry/fsimpl/fuse/connection.go | 4 + pkg/sentry/fsimpl/fuse/dev.go | 13 +- pkg/sentry/fsimpl/fuse/fusefs.go | 10 +- pkg/sentry/fsimpl/fuse/read_write.go | 90 ++++++++++ pkg/sentry/fsimpl/fuse/regular_file.go | 105 ++++++++++++ test/fuse/BUILD | 5 + test/fuse/linux/BUILD | 13 ++ test/fuse/linux/fuse_base.cc | 13 +- test/fuse/linux/fuse_base.h | 7 +- test/fuse/linux/write_test.cc | 303 +++++++++++++++++++++++++++++++++ 11 files changed, 592 insertions(+), 38 deletions(-) create mode 100644 test/fuse/linux/write_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index adc04dbcc..e49a92fb2 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -124,32 +124,6 @@ type FUSEHeaderOut struct { Unique FUSEOpID } -// FUSEWriteIn is the header written by a daemon when it makes a -// write request to the FUSE filesystem. -// -// +marshal -type FUSEWriteIn struct { - // Fh specifies the file handle that is being written to. - Fh uint64 - - // Offset is the offset of the write. - Offset uint64 - - // Size is the size of data being written. - Size uint32 - - // WriteFlags is the flags used during the write. - WriteFlags uint32 - - // LockOwner is the ID of the lock owner. - LockOwner uint64 - - // Flags is the flags for the request. - Flags uint32 - - _ uint32 -} - // FUSE_INIT flags, consistent with the ones in include/uapi/linux/fuse.h. // Our taget version is 7.23 but we have few implemented in advance. const ( @@ -427,6 +401,47 @@ type FUSEReadIn struct { _ uint32 } +// FUSEWriteIn is the first part of the payload of the +// request sent by the kernel to the daemon +// for FUSE_WRITE (struct for FUSE version >= 7.9). +// +// The second part of the payload is the +// binary bytes of the data to be written. +// +// +marshal +type FUSEWriteIn struct { + // Fh is the file handle in userspace. + Fh uint64 + + // Offset is the write offset. + Offset uint64 + + // Size is the number of bytes to write. + Size uint32 + + // ReadFlags for this FUSE_WRITE request. + WriteFlags uint32 + + // LockOwner is the id of the lock owner if there is one. + LockOwner uint64 + + // Flags for the underlying file. + Flags uint32 + + _ uint32 +} + +// FUSEWriteOut is the payload of the reply sent by the daemon to the kernel +// for a FUSE_WRITE request. +// +// +marshal +type FUSEWriteOut struct { + // Size is the number of bytes written. + Size uint32 + + _ uint32 +} + // FUSEReleaseIn is the request sent by the kernel to the daemon // when there is no more reference to a file. // diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index 5f20483b7..b3f981459 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -64,6 +64,10 @@ type Request struct { id linux.FUSEOpID hdr *linux.FUSEHeaderIn data []byte + + // payload for this request: extra bytes to write after + // the data slice. Used by FUSE_WRITE. + payload []byte } // Response represents an actual response from the server, including the diff --git a/pkg/sentry/fsimpl/fuse/dev.go b/pkg/sentry/fsimpl/fuse/dev.go index c53a38021..6022593d6 100644 --- a/pkg/sentry/fsimpl/fuse/dev.go +++ b/pkg/sentry/fsimpl/fuse/dev.go @@ -152,7 +152,7 @@ func (fd *DeviceFD) readLocked(ctx context.Context, dst usermem.IOSequence, opts for !fd.queue.Empty() { req = fd.queue.Front() - if int64(req.hdr.Len) <= dst.NumBytes() { + if int64(req.hdr.Len)+int64(len(req.payload)) <= dst.NumBytes() { break } @@ -191,6 +191,17 @@ func (fd *DeviceFD) readLocked(ctx context.Context, dst usermem.IOSequence, opts return 0, syserror.EIO } + if req.hdr.Opcode == linux.FUSE_WRITE { + written, err := dst.DropFirst(n).CopyOut(ctx, req.payload) + if err != nil { + return 0, err + } + if written != len(req.payload) { + return 0, syserror.EIO + } + n += int(written) + } + // Fully done with this req, remove it from the queue. fd.queue.Remove(req) if req.hdr.Opcode == linux.FUSE_RELEASE { diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index cfae9ed0d..8cf13dcb6 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -18,6 +18,7 @@ package fuse import ( "math" "strconv" + "sync" "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" @@ -228,13 +229,18 @@ type inode struct { kernfs.InodeNotSymlink kernfs.OrderedChildren - NodeID uint64 dentry kernfs.Dentry - locks vfs.FileLocks // the owning filesystem. fs is immutable. fs *filesystem + // metaDataMu protects the metadata of this inode. + metadataMu sync.Mutex + + NodeID uint64 + + locks vfs.FileLocks + // size of the file. size uint64 diff --git a/pkg/sentry/fsimpl/fuse/read_write.go b/pkg/sentry/fsimpl/fuse/read_write.go index 4ef8531dc..22a018e5e 100644 --- a/pkg/sentry/fsimpl/fuse/read_write.go +++ b/pkg/sentry/fsimpl/fuse/read_write.go @@ -150,3 +150,93 @@ func (fs *filesystem) ReadCallback(ctx context.Context, fd *regularFileFD, off u fs.conn.mu.Unlock() } } + +// Write sends FUSE_WRITE requests and return the bytes +// written according to the response. +// +// Preconditions: len(data) == size. +func (fs *filesystem) Write(ctx context.Context, fd *regularFileFD, off uint64, size uint32, data []byte) (uint32, error) { + t := kernel.TaskFromContext(ctx) + if t == nil { + log.Warningf("fusefs.Read: couldn't get kernel task from context") + return 0, syserror.EINVAL + } + + // One request cannnot exceed either maxWrite or maxPages. + maxWrite := uint32(fs.conn.maxPages) << usermem.PageShift + if maxWrite > fs.conn.maxWrite { + maxWrite = fs.conn.maxWrite + } + + // Reuse the same struct for unmarshalling to avoid unnecessary memory allocation. + in := linux.FUSEWriteIn{ + Fh: fd.Fh, + // TODO(gvisor.dev/issue/3245): file lock + LockOwner: 0, + // TODO(gvisor.dev/issue/3245): |= linux.FUSE_READ_LOCKOWNER + // TODO(gvisor.dev/issue/3237): |= linux.FUSE_WRITE_CACHE (not added yet) + WriteFlags: 0, + Flags: fd.statusFlags(), + } + + var written uint32 + + // This loop is intended for fragmented write where the bytes to write is + // larger than either the maxWrite or maxPages or when bigWrites is false. + // Unless a small value for max_write is explicitly used, this loop + // is expected to execute only once for the majority of the writes. + for written < size { + toWrite := size - written + + // Limit the write size to one page. + // Note that the bigWrites flag is obsolete, + // latest libfuse always sets it on. + if !fs.conn.bigWrites && toWrite > usermem.PageSize { + toWrite = usermem.PageSize + } + + // Limit the write size to maxWrite. + if toWrite > maxWrite { + toWrite = maxWrite + } + + in.Offset = off + uint64(written) + in.Size = toWrite + + req, err := fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(t.ThreadID()), fd.inode().NodeID, linux.FUSE_WRITE, &in) + if err != nil { + return 0, err + } + + req.payload = data[written : written+toWrite] + + // TODO(gvisor.dev/issue/3247): support async write. + + res, err := fs.conn.Call(t, req) + if err != nil { + return 0, err + } + if err := res.Error(); err != nil { + return 0, err + } + + out := linux.FUSEWriteOut{} + if err := res.UnmarshalPayload(&out); err != nil { + return 0, err + } + + // Write more than requested? EIO. + if out.Size > toWrite { + return 0, syserror.EIO + } + + written += out.Size + + // Break if short write. Not necessarily an error. + if out.Size != toWrite { + break + } + } + + return written, nil +} diff --git a/pkg/sentry/fsimpl/fuse/regular_file.go b/pkg/sentry/fsimpl/fuse/regular_file.go index 37ce4e268..193e77392 100644 --- a/pkg/sentry/fsimpl/fuse/regular_file.go +++ b/pkg/sentry/fsimpl/fuse/regular_file.go @@ -123,3 +123,108 @@ func (fd *regularFileFD) Read(ctx context.Context, dst usermem.IOSequence, opts fd.offMu.Unlock() return n, err } + +// PWrite implements vfs.FileDescriptionImpl.PWrite. +func (fd *regularFileFD) PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) { + n, _, err := fd.pwrite(ctx, src, offset, opts) + return n, err +} + +// Write implements vfs.FileDescriptionImpl.Write. +func (fd *regularFileFD) Write(ctx context.Context, src usermem.IOSequence, opts vfs.WriteOptions) (int64, error) { + fd.offMu.Lock() + n, off, err := fd.pwrite(ctx, src, fd.off, opts) + fd.off = off + fd.offMu.Unlock() + return n, err +} + +// pwrite returns the number of bytes written, final offset and error. The +// final offset should be ignored by PWrite. +func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (written, finalOff int64, err error) { + if offset < 0 { + return 0, offset, syserror.EINVAL + } + + // Check that flags are supported. + // + // TODO(gvisor.dev/issue/2601): Support select preadv2 flags. + if opts.Flags&^linux.RWF_HIPRI != 0 { + return 0, offset, syserror.EOPNOTSUPP + } + + inode := fd.inode() + inode.metadataMu.Lock() + defer inode.metadataMu.Unlock() + + // If the file is opened with O_APPEND, update offset to file size. + // Note: since our Open() implements the interface of kernfs, + // and kernfs currently does not support O_APPEND, this will never + // be true before we switch out from kernfs. + if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 { + // Locking inode.metadataMu is sufficient for reading size + offset = int64(inode.size) + } + + srclen := src.NumBytes() + + if srclen > math.MaxUint32 { + // FUSE only supports uint32 for size. + // Overflow. + return 0, offset, syserror.EINVAL + } + if end := offset + srclen; end < offset { + // Overflow. + return 0, offset, syserror.EINVAL + } + + srclen, err = vfs.CheckLimit(ctx, offset, srclen) + if err != nil { + return 0, offset, err + } + + if srclen == 0 { + // Return before causing any side effects. + return 0, offset, nil + } + + src = src.TakeFirst64(srclen) + + // TODO(gvisor.dev/issue/3237): Add cache support: + // buffer cache. Ideally we write from src to our buffer cache first. + // The slice passed to fs.Write() should be a slice from buffer cache. + data := make([]byte, srclen) + // Reason for making a copy here: connection.Call() blocks on kerneltask, + // which in turn acquires mm.activeMu lock. Functions like CopyInTo() will + // attemp to acquire the mm.activeMu lock as well -> deadlock. + // We must finish reading from the userspace memory before + // t.Block() deactivates it. + cp, err := src.CopyIn(ctx, data) + if err != nil { + return 0, offset, err + } + if int64(cp) != srclen { + return 0, offset, syserror.EIO + } + + n, err := fd.inode().fs.Write(ctx, fd, uint64(offset), uint32(srclen), data) + if err != nil { + return 0, offset, err + } + + if n == 0 { + // We have checked srclen != 0 previously. + // If err == nil, then it's a short write and we return EIO. + return 0, offset, syserror.EIO + } + + written = int64(n) + finalOff = offset + written + + if finalOff > int64(inode.size) { + atomic.StoreUint64(&inode.size, uint64(finalOff)) + atomic.AddUint64(&inode.fs.conn.attributeVersion, 1) + } + + return +} diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 1a6b5b516..55ad98748 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -42,6 +42,11 @@ syscall_test( test = "//test/fuse/linux:read_test", ) +syscall_test( + fuse = "True", + test = "//test/fuse/linux:write_test", +) + syscall_test( fuse = "True", test = "//test/fuse/linux:rmdir_test", diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index 2bb956af9..cc5bd560e 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -154,6 +154,19 @@ cc_binary( ], ) +cc_binary( + name = "write_test", + testonly = 1, + srcs = ["write_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_binary( name = "create_test", testonly = 1, diff --git a/test/fuse/linux/fuse_base.cc b/test/fuse/linux/fuse_base.cc index e3c6b585c..a033db117 100644 --- a/test/fuse/linux/fuse_base.cc +++ b/test/fuse/linux/fuse_base.cc @@ -164,7 +164,8 @@ void FuseTest::UnmountFuse() { } // Consumes the first FUSE request and returns the corresponding PosixError. -PosixError FuseTest::ServerConsumeFuseInit() { +PosixError FuseTest::ServerConsumeFuseInit( + const struct fuse_init_out* out_payload) { std::vector buf(FUSE_MIN_READ_BUFFER); RETURN_ERROR_IF_SYSCALL_FAIL( RetryEINTR(read)(dev_fd_, buf.data(), buf.size())); @@ -176,10 +177,8 @@ PosixError FuseTest::ServerConsumeFuseInit() { }; // Returns a fake fuse_init_out with 7.0 version to avoid ECONNREFUSED // error in the initialization of FUSE connection. - struct fuse_init_out out_payload = { - .major = 7, - }; - auto iov_out = FuseGenerateIovecs(out_header, out_payload); + auto iov_out = FuseGenerateIovecs( + out_header, *const_cast(out_payload)); RETURN_ERROR_IF_SYSCALL_FAIL( RetryEINTR(writev)(dev_fd_, iov_out.data(), iov_out.size())); @@ -244,7 +243,7 @@ void FuseTest::ServerFuseLoop() { // becomes testing thread and the child thread becomes the FUSE server running // in background. These 2 threads are connected via socketpair. sock_[0] is // opened in testing thread and sock_[1] is opened in the FUSE server. -void FuseTest::SetUpFuseServer() { +void FuseTest::SetUpFuseServer(const struct fuse_init_out* payload) { ASSERT_THAT(socketpair(AF_UNIX, SOCK_STREAM, 0, sock_), SyscallSucceeds()); switch (fork()) { @@ -261,7 +260,7 @@ void FuseTest::SetUpFuseServer() { // Begin child thread, i.e. the FUSE server. ASSERT_THAT(close(sock_[0]), SyscallSucceeds()); - ServerCompleteWith(ServerConsumeFuseInit().ok()); + ServerCompleteWith(ServerConsumeFuseInit(payload).ok()); ServerFuseLoop(); _exit(0); } diff --git a/test/fuse/linux/fuse_base.h b/test/fuse/linux/fuse_base.h index 452748d6d..6ad296ca2 100644 --- a/test/fuse/linux/fuse_base.h +++ b/test/fuse/linux/fuse_base.h @@ -33,6 +33,8 @@ namespace testing { constexpr char kMountOpts[] = "rootmode=755,user_id=0,group_id=0"; +constexpr struct fuse_init_out kDefaultFUSEInitOutPayload = {.major = 7}; + // Internal commands used to communicate between testing thread and the FUSE // server. See test/fuse/README.md for further detail. enum class FuseTestCmd { @@ -171,7 +173,8 @@ class FuseTest : public ::testing::Test { void MountFuse(const char* mountOpts = kMountOpts); // Creates a socketpair for communication and forks FUSE server. - void SetUpFuseServer(); + void SetUpFuseServer( + const struct fuse_init_out* payload = &kDefaultFUSEInitOutPayload); // Unmounts the mountpoint of the FUSE server. void UnmountFuse(); @@ -194,7 +197,7 @@ class FuseTest : public ::testing::Test { // Consumes the first FUSE request when mounting FUSE. Replies with a // response with empty payload. - PosixError ServerConsumeFuseInit(); + PosixError ServerConsumeFuseInit(const struct fuse_init_out* payload); // A command switch that dispatch different FuseTestCmd to its handler. void ServerHandleCommand(); diff --git a/test/fuse/linux/write_test.cc b/test/fuse/linux/write_test.cc new file mode 100644 index 000000000..e7a1aff13 --- /dev/null +++ b/test/fuse/linux/write_test.cc @@ -0,0 +1,303 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class WriteTest : public FuseTest { + void SetUp() override { + FuseTest::SetUp(); + test_file_path_ = JoinPath(mount_point_.path().c_str(), test_file_); + } + + // TearDown overrides the parent's function + // to skip checking the unconsumed release request at the end. + void TearDown() override { UnmountFuse(); } + + protected: + const std::string test_file_ = "test_file"; + const mode_t test_file_mode_ = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO; + const uint64_t test_fh_ = 1; + const uint32_t open_flag_ = O_RDWR; + + std::string test_file_path_; + + PosixErrorOr OpenTestFile(const std::string &path, + uint64_t size = 512) { + SetServerInodeLookup(test_file_, test_file_mode_, size); + + struct fuse_out_header out_header_open = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_open_out), + }; + struct fuse_open_out out_payload_open = { + .fh = test_fh_, + .open_flags = open_flag_, + }; + auto iov_out_open = FuseGenerateIovecs(out_header_open, out_payload_open); + SetServerResponse(FUSE_OPEN, iov_out_open); + + auto res = Open(path.c_str(), open_flag_); + if (res.ok()) { + SkipServerActualRequest(); + } + return res; + } +}; + +class WriteTestSmallMaxWrite : public WriteTest { + void SetUp() override { + MountFuse(); + SetUpFuseServer(&fuse_init_payload); + test_file_path_ = JoinPath(mount_point_.path().c_str(), test_file_); + } + + protected: + const static uint32_t max_write_ = 4096; + constexpr static struct fuse_init_out fuse_init_payload = { + .major = 7, + .max_write = max_write_, + }; + + const uint32_t size_fragment = max_write_; +}; + +TEST_F(WriteTest, WriteNormal) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Prepare for the write. + const int n_write = 10; + struct fuse_out_header out_header_write = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_write_out), + }; + struct fuse_write_out out_payload_write = { + .size = n_write, + }; + auto iov_out_write = FuseGenerateIovecs(out_header_write, out_payload_write); + SetServerResponse(FUSE_WRITE, iov_out_write); + + // Issue the write. + std::vector buf(n_write); + RandomizeBuffer(buf.data(), buf.size()); + EXPECT_THAT(write(fd.get(), buf.data(), n_write), + SyscallSucceedsWithValue(n_write)); + + // Check the write request. + struct fuse_in_header in_header_write; + struct fuse_write_in in_payload_write; + std::vector payload_buf(n_write); + auto iov_in_write = + FuseGenerateIovecs(in_header_write, in_payload_write, payload_buf); + GetServerActualRequest(iov_in_write); + + EXPECT_EQ(in_payload_write.fh, test_fh_); + EXPECT_EQ(in_header_write.len, + sizeof(in_header_write) + sizeof(in_payload_write)); + EXPECT_EQ(in_header_write.opcode, FUSE_WRITE); + EXPECT_EQ(in_payload_write.offset, 0); + EXPECT_EQ(in_payload_write.size, n_write); + EXPECT_EQ(buf, payload_buf); +} + +TEST_F(WriteTest, WriteShort) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Prepare for the write. + const int n_write = 10, n_written = 5; + struct fuse_out_header out_header_write = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_write_out), + }; + struct fuse_write_out out_payload_write = { + .size = n_written, + }; + auto iov_out_write = FuseGenerateIovecs(out_header_write, out_payload_write); + SetServerResponse(FUSE_WRITE, iov_out_write); + + // Issue the write. + std::vector buf(n_write); + RandomizeBuffer(buf.data(), buf.size()); + EXPECT_THAT(write(fd.get(), buf.data(), n_write), + SyscallSucceedsWithValue(n_written)); + + // Check the write request. + struct fuse_in_header in_header_write; + struct fuse_write_in in_payload_write; + std::vector payload_buf(n_write); + auto iov_in_write = + FuseGenerateIovecs(in_header_write, in_payload_write, payload_buf); + GetServerActualRequest(iov_in_write); + + EXPECT_EQ(in_payload_write.fh, test_fh_); + EXPECT_EQ(in_header_write.len, + sizeof(in_header_write) + sizeof(in_payload_write)); + EXPECT_EQ(in_header_write.opcode, FUSE_WRITE); + EXPECT_EQ(in_payload_write.offset, 0); + EXPECT_EQ(in_payload_write.size, n_write); + EXPECT_EQ(buf, payload_buf); +} + +TEST_F(WriteTest, WriteShortZero) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Prepare for the write. + const int n_write = 10; + struct fuse_out_header out_header_write = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_write_out), + }; + struct fuse_write_out out_payload_write = { + .size = 0, + }; + auto iov_out_write = FuseGenerateIovecs(out_header_write, out_payload_write); + SetServerResponse(FUSE_WRITE, iov_out_write); + + // Issue the write. + std::vector buf(n_write); + RandomizeBuffer(buf.data(), buf.size()); + EXPECT_THAT(write(fd.get(), buf.data(), n_write), SyscallFailsWithErrno(EIO)); + + // Check the write request. + struct fuse_in_header in_header_write; + struct fuse_write_in in_payload_write; + std::vector payload_buf(n_write); + auto iov_in_write = + FuseGenerateIovecs(in_header_write, in_payload_write, payload_buf); + GetServerActualRequest(iov_in_write); + + EXPECT_EQ(in_payload_write.fh, test_fh_); + EXPECT_EQ(in_header_write.len, + sizeof(in_header_write) + sizeof(in_payload_write)); + EXPECT_EQ(in_header_write.opcode, FUSE_WRITE); + EXPECT_EQ(in_payload_write.offset, 0); + EXPECT_EQ(in_payload_write.size, n_write); + EXPECT_EQ(buf, payload_buf); +} + +TEST_F(WriteTest, WriteZero) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_)); + + // Issue the write. + std::vector buf(0); + EXPECT_THAT(write(fd.get(), buf.data(), 0), SyscallSucceedsWithValue(0)); +} + +TEST_F(WriteTest, PWrite) { + const int file_size = 512; + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_, file_size)); + + // Prepare for the write. + const int n_write = 10; + struct fuse_out_header out_header_write = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_write_out), + }; + struct fuse_write_out out_payload_write = { + .size = n_write, + }; + auto iov_out_write = FuseGenerateIovecs(out_header_write, out_payload_write); + SetServerResponse(FUSE_WRITE, iov_out_write); + + // Issue the write. + std::vector buf(n_write); + RandomizeBuffer(buf.data(), buf.size()); + const int offset_write = file_size >> 1; + EXPECT_THAT(pwrite(fd.get(), buf.data(), n_write, offset_write), + SyscallSucceedsWithValue(n_write)); + + // Check the write request. + struct fuse_in_header in_header_write; + struct fuse_write_in in_payload_write; + std::vector payload_buf(n_write); + auto iov_in_write = + FuseGenerateIovecs(in_header_write, in_payload_write, payload_buf); + GetServerActualRequest(iov_in_write); + + EXPECT_EQ(in_payload_write.fh, test_fh_); + EXPECT_EQ(in_header_write.len, + sizeof(in_header_write) + sizeof(in_payload_write)); + EXPECT_EQ(in_header_write.opcode, FUSE_WRITE); + EXPECT_EQ(in_payload_write.offset, offset_write); + EXPECT_EQ(in_payload_write.size, n_write); + EXPECT_EQ(buf, payload_buf); +} + +TEST_F(WriteTestSmallMaxWrite, WriteSmallMaxWrie) { + const int n_fragment = 10; + const int n_write = size_fragment * n_fragment; + + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenTestFile(test_file_path_, n_write)); + + // Prepare for the write. + struct fuse_out_header out_header_write = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_write_out), + }; + struct fuse_write_out out_payload_write = { + .size = size_fragment, + }; + auto iov_out_write = FuseGenerateIovecs(out_header_write, out_payload_write); + + for (int i = 0; i < n_fragment; ++i) { + SetServerResponse(FUSE_WRITE, iov_out_write); + } + + // Issue the write. + std::vector buf(n_write); + RandomizeBuffer(buf.data(), buf.size()); + EXPECT_THAT(write(fd.get(), buf.data(), n_write), + SyscallSucceedsWithValue(n_write)); + + ASSERT_EQ(GetServerNumUnsentResponses(), 0); + ASSERT_EQ(GetServerNumUnconsumedRequests(), n_fragment); + + // Check the write request. + struct fuse_in_header in_header_write; + struct fuse_write_in in_payload_write; + std::vector payload_buf(size_fragment); + auto iov_in_write = + FuseGenerateIovecs(in_header_write, in_payload_write, payload_buf); + + for (int i = 0; i < n_fragment; ++i) { + GetServerActualRequest(iov_in_write); + + EXPECT_EQ(in_payload_write.fh, test_fh_); + EXPECT_EQ(in_header_write.len, + sizeof(in_header_write) + sizeof(in_payload_write)); + EXPECT_EQ(in_header_write.opcode, FUSE_WRITE); + EXPECT_EQ(in_payload_write.offset, i * size_fragment); + EXPECT_EQ(in_payload_write.size, size_fragment); + + auto it = buf.begin() + i * size_fragment; + EXPECT_EQ(std::vector(it, it + size_fragment), payload_buf); + } +} + +} // namespace + +} // namespace testing +} // namespace gvisor \ No newline at end of file -- cgit v1.2.3 From 4181e8c97482a3c787c2b508e6d75a21323ba515 Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Wed, 9 Sep 2020 09:16:09 -0700 Subject: 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 --- pkg/abi/linux/fuse.go | 5 ++ pkg/sentry/fsimpl/fuse/fusefs.go | 21 +++---- pkg/sentry/fsimpl/fuse/regular_file.go | 2 +- test/fuse/linux/BUILD | 4 +- test/fuse/linux/stat_test.cc | 112 ++++++++++++++++++++++++++++++--- test/util/fuse_util.cc | 12 ++-- test/util/fuse_util.h | 5 +- 7 files changed, 133 insertions(+), 28 deletions(-) (limited to 'pkg/abi') 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 #include #include +#include #include #include #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(attr.size), + .st_blksize = attr.blksize, + .st_blocks = static_cast(attr.blocks), + .st_atim = (struct timespec){.tv_sec = static_cast(attr.atime), + .tv_nsec = attr.atimensec}, + .st_mtim = (struct timespec){.tv_sec = static_cast(attr.mtime), + .tv_nsec = attr.mtimensec}, + .st_ctim = (struct timespec){.tv_sec = static_cast(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 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 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 -- cgit v1.2.3 From bf8efe8cdf4b6af50ec89cda37342cf51c8b50b4 Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Wed, 9 Sep 2020 10:44:09 -0700 Subject: Implement FUSE_SETATTR This commit implements FUSE_SETATTR command. When a system call modifies the metadata of a regular file or a folder by chown(2), chmod(2), truncate(2), utime(2), or utimes(2), they should be translated to corresponding FUSE_SETATTR command and sent to the FUSE server. Fixes #3332 --- pkg/abi/linux/fuse.go | 67 ++++++ pkg/sentry/fsimpl/fuse/file.go | 2 +- pkg/sentry/fsimpl/fuse/fusefs.go | 83 ++++++- pkg/sentry/fsimpl/kernfs/inode_impl_util.go | 7 + test/fuse/BUILD | 5 + test/fuse/linux/BUILD | 16 ++ test/fuse/linux/setstat_test.cc | 338 ++++++++++++++++++++++++++++ 7 files changed, 516 insertions(+), 2 deletions(-) create mode 100644 test/fuse/linux/setstat_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index f0bef1e8e..fcc957bfe 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -749,3 +749,70 @@ func (r *FUSEDirent) UnmarshalBytes(src []byte) { name.UnmarshalBytes(src[:r.Meta.NameLen]) r.Name = string(name) } + +// FATTR_* consts are the attribute flags defined in include/uapi/linux/fuse.h. +// These should be or-ed together for setattr to know what has been changed. +const ( + FATTR_MODE = (1 << 0) + FATTR_UID = (1 << 1) + FATTR_GID = (1 << 2) + FATTR_SIZE = (1 << 3) + FATTR_ATIME = (1 << 4) + FATTR_MTIME = (1 << 5) + FATTR_FH = (1 << 6) + FATTR_ATIME_NOW = (1 << 7) + FATTR_MTIME_NOW = (1 << 8) + FATTR_LOCKOWNER = (1 << 9) + FATTR_CTIME = (1 << 10) +) + +// FUSESetAttrIn is the request sent by the kernel to the daemon, +// to set the attribute(s) of a file. +// +// +marshal +type FUSESetAttrIn struct { + // Valid indicates which attributes are modified by this request. + Valid uint32 + + _ uint32 + + // Fh is used to identify the file if FATTR_FH is set in Valid. + Fh uint64 + + // Size is the size that the request wants to change to. + Size uint64 + + // LockOwner is the owner of the lock that the request wants to change to. + LockOwner uint64 + + // Atime is the access time that the request wants to change to. + Atime uint64 + + // Mtime is the modification time that the request wants to change to. + Mtime uint64 + + // Ctime is the status change time that the request wants to change to. + Ctime uint64 + + // AtimeNsec is the nano second part of Atime. + AtimeNsec uint32 + + // MtimeNsec is the nano second part of Mtime. + MtimeNsec uint32 + + // CtimeNsec is the nano second part of Ctime. + CtimeNsec uint32 + + // Mode is the file mode that the request wants to change to. + Mode uint32 + + _ uint32 + + // UID is the user ID of the owner that the request wants to change to. + UID uint32 + + // GID is the group ID of the owner that the request wants to change to. + GID uint32 + + _ uint32 +} diff --git a/pkg/sentry/fsimpl/fuse/file.go b/pkg/sentry/fsimpl/fuse/file.go index 186ec2362..15c0e3f41 100644 --- a/pkg/sentry/fsimpl/fuse/file.go +++ b/pkg/sentry/fsimpl/fuse/file.go @@ -123,5 +123,5 @@ 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 { creds := auth.CredentialsFromContext(ctx) - return fd.inode().SetStat(ctx, fd.inode().fs.VFSFilesystem(), creds, opts) + return fd.inode().setAttr(ctx, fd.inode().fs.VFSFilesystem(), creds, opts, true, fd.Fh) } diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 821048d87..572245303 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -660,7 +660,7 @@ func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOp } // Set the metadata of kernfs.InodeAttrs. - if err := i.SetStat(ctx, fs, creds, vfs.SetStatOptions{ + if err := i.SetInodeStat(ctx, fs, creds, vfs.SetStatOptions{ Stat: statFromFUSEAttr(out.Attr, linux.STATX_ALL, i.fs.devMinor), }); err != nil { return linux.FUSEAttr{}, err @@ -703,3 +703,84 @@ func (i *inode) StatFS(ctx context.Context, fs *vfs.Filesystem) (linux.Statfs, e // TODO(gvisor.dev/issues/3413): Complete the implementation of statfs. return vfs.GenericStatFS(linux.FUSE_SUPER_MAGIC), nil } + +// fattrMaskFromStats converts vfs.SetStatOptions.Stat.Mask to linux stats mask +// aligned with the attribute mask defined in include/linux/fs.h. +func fattrMaskFromStats(mask uint32) uint32 { + var fuseAttrMask uint32 + maskMap := map[uint32]uint32{ + linux.STATX_MODE: linux.FATTR_MODE, + linux.STATX_UID: linux.FATTR_UID, + linux.STATX_GID: linux.FATTR_GID, + linux.STATX_SIZE: linux.FATTR_SIZE, + linux.STATX_ATIME: linux.FATTR_ATIME, + linux.STATX_MTIME: linux.FATTR_MTIME, + linux.STATX_CTIME: linux.FATTR_CTIME, + } + for statxMask, fattrMask := range maskMap { + if mask&statxMask != 0 { + fuseAttrMask |= fattrMask + } + } + return fuseAttrMask +} + +// SetStat implements kernfs.Inode.SetStat. +func (i *inode) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *auth.Credentials, opts vfs.SetStatOptions) error { + return i.setAttr(ctx, fs, creds, opts, false, 0) +} + +func (i *inode) setAttr(ctx context.Context, fs *vfs.Filesystem, creds *auth.Credentials, opts vfs.SetStatOptions, useFh bool, fh uint64) error { + conn := i.fs.conn + task := kernel.TaskFromContext(ctx) + if task == nil { + log.Warningf("couldn't get kernel task from context") + return syserror.EINVAL + } + + // We should retain the original file type when assigning new mode. + fileType := uint16(i.Mode()) & linux.S_IFMT + fattrMask := fattrMaskFromStats(opts.Stat.Mask) + if useFh { + fattrMask |= linux.FATTR_FH + } + in := linux.FUSESetAttrIn{ + Valid: fattrMask, + Fh: fh, + Size: opts.Stat.Size, + Atime: uint64(opts.Stat.Atime.Sec), + Mtime: uint64(opts.Stat.Mtime.Sec), + Ctime: uint64(opts.Stat.Ctime.Sec), + AtimeNsec: opts.Stat.Atime.Nsec, + MtimeNsec: opts.Stat.Mtime.Nsec, + CtimeNsec: opts.Stat.Ctime.Nsec, + Mode: uint32(fileType | opts.Stat.Mode), + UID: opts.Stat.UID, + GID: opts.Stat.GID, + } + req, err := conn.NewRequest(creds, uint32(task.ThreadID()), i.NodeID, linux.FUSE_SETATTR, &in) + if err != nil { + return err + } + + res, err := conn.Call(task, req) + if err != nil { + return err + } + if err := res.Error(); err != nil { + return err + } + out := linux.FUSEGetAttrOut{} + if err := res.UnmarshalPayload(&out); err != nil { + return err + } + + // Set the metadata of kernfs.InodeAttrs. + if err := i.SetInodeStat(ctx, fs, creds, vfs.SetStatOptions{ + Stat: statFromFUSEAttr(out.Attr, linux.STATX_ALL, i.fs.devMinor), + }); err != nil { + return err + } + + return nil +} diff --git a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go index 53758a4b8..c2109cf76 100644 --- a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go +++ b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go @@ -256,6 +256,13 @@ func (a *InodeAttrs) Stat(context.Context, *vfs.Filesystem, vfs.StatOptions) (li // SetStat implements Inode.SetStat. func (a *InodeAttrs) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *auth.Credentials, opts vfs.SetStatOptions) error { + return a.SetInodeStat(ctx, fs, creds, opts) +} + +// SetInodeStat sets the corresponding attributes from opts to InodeAttrs. +// This function can be used by other kernfs-based filesystem implementation to +// sets the unexported attributes into kernfs.InodeAttrs. +func (a *InodeAttrs) SetInodeStat(ctx context.Context, fs *vfs.Filesystem, creds *auth.Credentials, opts vfs.SetStatOptions) error { if opts.Stat.Mask == 0 { return nil } diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 55ad98748..29b9a9d93 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -62,6 +62,11 @@ syscall_test( test = "//test/fuse/linux:create_test", ) +syscall_test( + fuse = "True", + test = "//test/fuse/linux:setstat_test", +) + syscall_test( size = "large", add_overlay = True, diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index 7a3e52fad..7ecd6d8cb 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -100,6 +100,22 @@ cc_binary( ], ) +cc_binary( + name = "setstat_test", + testonly = 1, + srcs = ["setstat_test.cc"], + deps = [ + gtest, + ":fuse_fd_util", + "//test/util:cleanup", + "//test/util:fs_util", + "//test/util:fuse_util", + "//test/util:temp_umask", + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_binary( name = "rmdir_test", testonly = 1, diff --git a/test/fuse/linux/setstat_test.cc b/test/fuse/linux/setstat_test.cc new file mode 100644 index 000000000..68301c775 --- /dev/null +++ b/test/fuse/linux/setstat_test.cc @@ -0,0 +1,338 @@ +// 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 +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.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" + +namespace gvisor { +namespace testing { + +namespace { + +class SetStatTest : public FuseFdTest { + public: + void SetUp() override { + FuseFdTest::SetUp(); + test_dir_path_ = JoinPath(mount_point_.path(), test_dir_); + test_file_path_ = JoinPath(mount_point_.path(), test_file_); + } + + protected: + const uint64_t fh = 23; + const std::string test_dir_ = "testdir"; + const std::string test_file_ = "testfile"; + const mode_t test_dir_mode_ = S_IFDIR | S_IRUSR | S_IWUSR | S_IXUSR; + const mode_t test_file_mode_ = S_IFREG | S_IRUSR | S_IWUSR | S_IXUSR; + + std::string test_dir_path_; + std::string test_file_path_; +}; + +TEST_F(SetStatTest, ChmodDir) { + // Set up fixture. + SetServerInodeLookup(test_dir_, test_dir_mode_); + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out), + .error = 0, + }; + mode_t set_mode = S_IRGRP | S_IWGRP | S_IXGRP; + struct fuse_attr_out out_payload = { + .attr = DefaultFuseAttr(set_mode, 2), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SETATTR, iov_out); + + // Make syscall. + EXPECT_THAT(chmod(test_dir_path_.c_str(), set_mode), SyscallSucceeds()); + + // Check FUSE request. + struct fuse_in_header in_header; + struct fuse_setattr_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_SETATTR); + EXPECT_EQ(in_header.uid, 0); + EXPECT_EQ(in_header.gid, 0); + EXPECT_EQ(in_payload.valid, FATTR_MODE); + EXPECT_EQ(in_payload.mode, S_IFDIR | set_mode); +} + +TEST_F(SetStatTest, ChownDir) { + // Set up fixture. + SetServerInodeLookup(test_dir_, test_dir_mode_); + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out), + .error = 0, + }; + struct fuse_attr_out out_payload = { + .attr = DefaultFuseAttr(test_dir_mode_, 2), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SETATTR, iov_out); + + // Make syscall. + EXPECT_THAT(chown(test_dir_path_.c_str(), 1025, 1025), SyscallSucceeds()); + + // Check FUSE request. + struct fuse_in_header in_header; + struct fuse_setattr_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_SETATTR); + EXPECT_EQ(in_header.uid, 0); + EXPECT_EQ(in_header.gid, 0); + EXPECT_EQ(in_payload.valid, FATTR_UID | FATTR_GID); + EXPECT_EQ(in_payload.uid, 1025); + EXPECT_EQ(in_payload.gid, 1025); +} + +TEST_F(SetStatTest, TruncateFile) { + // Set up fixture. + SetServerInodeLookup(test_file_, test_file_mode_); + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out), + .error = 0, + }; + struct fuse_attr_out out_payload = { + .attr = DefaultFuseAttr(S_IFREG | S_IRUSR | S_IWUSR, 2), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SETATTR, iov_out); + + // Make syscall. + EXPECT_THAT(truncate(test_file_path_.c_str(), 321), SyscallSucceeds()); + + // Check FUSE request. + struct fuse_in_header in_header; + struct fuse_setattr_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_SETATTR); + EXPECT_EQ(in_header.uid, 0); + EXPECT_EQ(in_header.gid, 0); + EXPECT_EQ(in_payload.valid, FATTR_SIZE); + EXPECT_EQ(in_payload.size, 321); +} + +TEST_F(SetStatTest, UtimeFile) { + // Set up fixture. + SetServerInodeLookup(test_file_, test_file_mode_); + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out), + .error = 0, + }; + struct fuse_attr_out out_payload = { + .attr = DefaultFuseAttr(S_IFREG | S_IRUSR | S_IWUSR, 2), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SETATTR, iov_out); + + // Make syscall. + time_t expected_atime = 1597159766, expected_mtime = 1597159765; + struct utimbuf times = { + .actime = expected_atime, + .modtime = expected_mtime, + }; + EXPECT_THAT(utime(test_file_path_.c_str(), ×), SyscallSucceeds()); + + // Check FUSE request. + struct fuse_in_header in_header; + struct fuse_setattr_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_SETATTR); + EXPECT_EQ(in_header.uid, 0); + EXPECT_EQ(in_header.gid, 0); + EXPECT_EQ(in_payload.valid, FATTR_ATIME | FATTR_MTIME); + EXPECT_EQ(in_payload.atime, expected_atime); + EXPECT_EQ(in_payload.mtime, expected_mtime); +} + +TEST_F(SetStatTest, UtimesFile) { + // Set up fixture. + SetServerInodeLookup(test_file_, test_file_mode_); + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out), + .error = 0, + }; + struct fuse_attr_out out_payload = { + .attr = DefaultFuseAttr(test_file_mode_, 2), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SETATTR, iov_out); + + // Make syscall. + struct timeval expected_times[2] = { + { + .tv_sec = 1597159766, + .tv_usec = 234945, + }, + { + .tv_sec = 1597159765, + .tv_usec = 232341, + }, + }; + EXPECT_THAT(utimes(test_file_path_.c_str(), expected_times), + SyscallSucceeds()); + + // Check FUSE request. + struct fuse_in_header in_header; + struct fuse_setattr_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_SETATTR); + EXPECT_EQ(in_header.uid, 0); + EXPECT_EQ(in_header.gid, 0); + EXPECT_EQ(in_payload.valid, FATTR_ATIME | FATTR_MTIME); + EXPECT_EQ(in_payload.atime, expected_times[0].tv_sec); + EXPECT_EQ(in_payload.atimensec, expected_times[0].tv_usec * 1000); + EXPECT_EQ(in_payload.mtime, expected_times[1].tv_sec); + EXPECT_EQ(in_payload.mtimensec, expected_times[1].tv_usec * 1000); +} + +TEST_F(SetStatTest, FtruncateFile) { + // Set up fixture. + SetServerInodeLookup(test_file_, test_file_mode_); + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenPath(test_file_path_, O_RDWR, fh)); + auto close_fd = CloseFD(fd); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out), + .error = 0, + }; + struct fuse_attr_out out_payload = { + .attr = DefaultFuseAttr(test_file_mode_, 2), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SETATTR, iov_out); + + // Make syscall. + EXPECT_THAT(ftruncate(fd.get(), 321), SyscallSucceeds()); + + // Check FUSE request. + struct fuse_in_header in_header; + struct fuse_setattr_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_SETATTR); + EXPECT_EQ(in_header.uid, 0); + EXPECT_EQ(in_header.gid, 0); + EXPECT_EQ(in_payload.valid, FATTR_SIZE | FATTR_FH); + EXPECT_EQ(in_payload.fh, fh); + EXPECT_EQ(in_payload.size, 321); +} + +TEST_F(SetStatTest, FchmodFile) { + // Set up fixture. + SetServerInodeLookup(test_file_, test_file_mode_); + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenPath(test_file_path_, O_RDWR, fh)); + auto close_fd = CloseFD(fd); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out), + .error = 0, + }; + mode_t set_mode = S_IROTH | S_IWOTH | S_IXOTH; + struct fuse_attr_out out_payload = { + .attr = DefaultFuseAttr(set_mode, 2), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SETATTR, iov_out); + + // Make syscall. + EXPECT_THAT(fchmod(fd.get(), set_mode), SyscallSucceeds()); + + // Check FUSE request. + struct fuse_in_header in_header; + struct fuse_setattr_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_SETATTR); + EXPECT_EQ(in_header.uid, 0); + EXPECT_EQ(in_header.gid, 0); + EXPECT_EQ(in_payload.valid, FATTR_MODE | FATTR_FH); + EXPECT_EQ(in_payload.fh, fh); + EXPECT_EQ(in_payload.mode, S_IFREG | set_mode); +} + +TEST_F(SetStatTest, FchownFile) { + // Set up fixture. + SetServerInodeLookup(test_file_, test_file_mode_); + auto fd = ASSERT_NO_ERRNO_AND_VALUE(OpenPath(test_file_path_, O_RDWR, fh)); + auto close_fd = CloseFD(fd); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header) + sizeof(struct fuse_attr_out), + .error = 0, + }; + struct fuse_attr_out out_payload = { + .attr = DefaultFuseAttr(S_IFREG | S_IRUSR | S_IWUSR | S_IXUSR, 2), + }; + auto iov_out = FuseGenerateIovecs(out_header, out_payload); + SetServerResponse(FUSE_SETATTR, iov_out); + + // Make syscall. + EXPECT_THAT(fchown(fd.get(), 1025, 1025), SyscallSucceeds()); + + // Check FUSE request. + struct fuse_in_header in_header; + struct fuse_setattr_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_SETATTR); + EXPECT_EQ(in_header.uid, 0); + EXPECT_EQ(in_header.gid, 0); + EXPECT_EQ(in_payload.valid, FATTR_UID | FATTR_GID | FATTR_FH); + EXPECT_EQ(in_payload.fh, fh); + EXPECT_EQ(in_payload.uid, 1025); + EXPECT_EQ(in_payload.gid, 1025); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From 2fbbe3b76864bc5b42f05d84166008b25e599bdb Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Wed, 9 Sep 2020 13:10:56 -0700 Subject: Add comments for exported attributes --- pkg/abi/linux/fuse.go | 56 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 13 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index fcc957bfe..ba3316ad6 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -252,22 +252,52 @@ type FUSEGetAttrIn struct { // // +marshal type FUSEAttr struct { - Ino uint64 - Size uint64 - Blocks uint64 - Atime uint64 - Mtime uint64 - Ctime uint64 + // Ino is the inode number of this file. + Ino uint64 + + // Size is the size of this file. + Size uint64 + + // Blocks is the number of the 512B blocks allocated by this file. + Blocks uint64 + + // Atime is the time of last access. + Atime uint64 + + // Mtime is the time of last modification. + Mtime uint64 + + // Ctime is the time of last status change. + Ctime uint64 + + // AtimeNsec is the nano second part of Atime. AtimeNsec uint32 + + // MtimeNsec is the nano second part of Mtime. MtimeNsec uint32 + + // CtimeNsec is the nano second part of Ctime. CtimeNsec uint32 - Mode uint32 - Nlink uint32 - UID uint32 - GID uint32 - Rdev uint32 - BlkSize uint32 - _ uint32 + + // Mode contains the file type and mode. + Mode uint32 + + // Nlink is the number of the hard links. + Nlink uint32 + + // UID is user ID of the owner. + UID uint32 + + // GID is group ID of the owner. + GID uint32 + + // Rdev is the device ID if this is a special file. + Rdev uint32 + + // BlkSize is the block size for filesystem I/O. + BlkSize uint32 + + _ uint32 } // FUSEGetAttrOut is the reply sent by the daemon to the kernel -- cgit v1.2.3 From 70cfea23776189575807f6aae28769bdfaa1e3fb Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Wed, 9 Sep 2020 16:44:35 -0700 Subject: Fix comments of TODO issues. --- pkg/abi/linux/fuse.go | 2 +- pkg/sentry/fsimpl/fuse/request_response.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index ba3316ad6..ca304af05 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -663,7 +663,7 @@ func (r *FUSEMkdirIn) SizeBytes() int { type FUSERmDirIn struct { marshal.StubMarshallable - // Name is a directory name to be looked up. + // Name is a directory name to be removed. Name string } diff --git a/pkg/sentry/fsimpl/fuse/request_response.go b/pkg/sentry/fsimpl/fuse/request_response.go index fd9a96197..70db50f38 100644 --- a/pkg/sentry/fsimpl/fuse/request_response.go +++ b/pkg/sentry/fsimpl/fuse/request_response.go @@ -122,7 +122,7 @@ func (conn *connection) NewRequest(creds *auth.Credentials, pid uint32, ino uint buf := make([]byte, hdr.Len) - // TODO(gVisor.dev/3698): Use the unsafe version once go_marshal is safe to use again. + // TODO(gVisor.dev/issue/3698): Use the unsafe version once go_marshal is safe to use again. hdr.MarshalBytes(buf[:hdrLen]) payload.MarshalBytes(buf[hdrLen:]) @@ -223,7 +223,7 @@ func (r *Response) UnmarshalPayload(m marshal.Marshallable) error { return nil } - // TODO(gVisor.dev/3698): Use the unsafe version once go_marshal is safe to use again. + // TODO(gVisor.dev/issue/3698): Use the unsafe version once go_marshal is safe to use again. m.UnmarshalBytes(r.data[hdrLen:]) return nil } -- cgit v1.2.3 From 2051260e8286b25ab39f3c1cb4614005236cbcc6 Mon Sep 17 00:00:00 2001 From: Boyuan He Date: Wed, 9 Sep 2020 17:13:18 -0700 Subject: Implement FUSE_UNLINK Fixes #3696 --- pkg/abi/linux/fuse.go | 23 ++++++++++ pkg/sentry/fsimpl/fuse/fusefs.go | 23 ++++++++++ pkg/sentry/fsimpl/kernfs/filesystem.go | 6 ++- pkg/sentry/fsimpl/kernfs/kernfs.go | 31 +++++++++++++ test/fuse/BUILD | 5 ++ test/fuse/linux/BUILD | 14 ++++++ test/fuse/linux/unlink_test.cc | 83 ++++++++++++++++++++++++++++++++++ 7 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 test/fuse/linux/unlink_test.cc (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index ca304af05..fdd22a13d 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -846,3 +846,26 @@ type FUSESetAttrIn struct { _ uint32 } + +// FUSEUnlinkIn is the request sent by the kernel to the daemon +// when trying to unlink a node. +// +// Dynamically-sized objects cannot be marshalled. +type FUSEUnlinkIn struct { + marshal.StubMarshallable + + // Name of the node to unlink. + Name string +} + +// MarshalBytes serializes r.name to the dst buffer, which should +// have size len(r.Name) + 1 and last byte set to 0. +func (r *FUSEUnlinkIn) MarshalBytes(buf []byte) { + copy(buf, r.Name) +} + +// SizeBytes is the size of the memory representation of FUSEUnlinkIn. +// 1 extra byte for null-terminated Name string. +func (r *FUSEUnlinkIn) SizeBytes() int { + return len(r.Name) + 1 +} diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index f4ed73c12..8e749bdad 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -455,6 +455,29 @@ func (i *inode) NewSymlink(ctx context.Context, name, target string) (*vfs.Dentr return i.newEntry(ctx, name, linux.S_IFLNK, linux.FUSE_SYMLINK, &in) } +// Unlink implements kernfs.Inode.Unlink. +func (i *inode) Unlink(ctx context.Context, name string, child *vfs.Dentry) error { + kernelTask := kernel.TaskFromContext(ctx) + if kernelTask == nil { + log.Warningf("fusefs.Inode.newEntry: couldn't get kernel task from context", i.NodeID) + return syserror.EINVAL + } + in := linux.FUSEUnlinkIn{Name: name} + req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.NodeID, linux.FUSE_UNLINK, &in) + if err != nil { + return err + } + res, err := i.fs.conn.Call(kernelTask, req) + if err != nil { + return err + } + // only return error, discard res. + if err := res.Error(); err != nil { + return err + } + return i.dentry.RemoveChildLocked(name, child) +} + // NewDir implements kernfs.Inode.NewDir. func (i *inode) NewDir(ctx context.Context, name string, opts vfs.MkdirOptions) (*vfs.Dentry, error) { in := linux.FUSEMkdirIn{ diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index 2823c3b1a..49f6a0f1d 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/pkg/sentry/fsimpl/kernfs/filesystem.go @@ -770,6 +770,10 @@ func (fs *Filesystem) SymlinkAt(ctx context.Context, rp *vfs.ResolvingPath, targ func (fs *Filesystem) UnlinkAt(ctx context.Context, rp *vfs.ResolvingPath) error { fs.mu.Lock() defer fs.mu.Unlock() + + // Store the name before walkExistingLocked as rp will be advanced past the + // name in the following call. + name := rp.Component() vfsd, _, err := fs.walkExistingLocked(ctx, rp) fs.processDeferredDecRefsLocked(ctx) if err != nil { @@ -795,7 +799,7 @@ func (fs *Filesystem) UnlinkAt(ctx context.Context, rp *vfs.ResolvingPath) error if err := virtfs.PrepareDeleteDentry(mntns, vfsd); err != nil { return err } - if err := parentDentry.inode.Unlink(ctx, rp.Component(), vfsd); err != nil { + if err := parentDentry.inode.Unlink(ctx, name, vfsd); err != nil { virtfs.AbortDeleteDentry(vfsd) return err } diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index 61189af25..163f26ceb 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -60,6 +60,7 @@ import ( "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/sync" + "gvisor.dev/gvisor/pkg/syserror" ) // Filesystem mostly implements vfs.FilesystemImpl for a generic in-memory @@ -267,6 +268,36 @@ func (d *Dentry) InsertChildLocked(name string, child *Dentry) { d.children[name] = child } +// RemoveChild removes child from the vfs dentry cache. This does not update the +// directory inode or modify the inode to be unlinked. So calling this on its own +// isn't sufficient to remove a child from a directory. +// +// Precondition: d must represent a directory inode. +func (d *Dentry) RemoveChild(name string, child *vfs.Dentry) error { + d.dirMu.Lock() + defer d.dirMu.Unlock() + return d.RemoveChildLocked(name, child) +} + +// RemoveChildLocked is equivalent to RemoveChild, with additional +// preconditions. +// +// Precondition: d.dirMu must be locked. +func (d *Dentry) RemoveChildLocked(name string, child *vfs.Dentry) error { + if !d.isDir() { + panic(fmt.Sprintf("RemoveChild called on non-directory Dentry: %+v.", d)) + } + c, ok := d.children[name] + if !ok { + return syserror.ENOENT + } + if &c.vfsd != child { + panic(fmt.Sprintf("Dentry hashed into inode doesn't match what vfs thinks! Child: %+v, vfs: %+v", c, child)) + } + delete(d.children, name) + return nil +} + // Inode returns the dentry's inode. func (d *Dentry) Inode() Inode { return d.inode diff --git a/test/fuse/BUILD b/test/fuse/BUILD index 29b9a9d93..dacfe0175 100644 --- a/test/fuse/BUILD +++ b/test/fuse/BUILD @@ -62,6 +62,11 @@ syscall_test( test = "//test/fuse/linux:create_test", ) +syscall_test( + fuse = "True", + test = "//test/fuse/linux:unlink_test", +) + syscall_test( fuse = "True", test = "//test/fuse/linux:setstat_test", diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index 7ecd6d8cb..7673252ec 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -214,3 +214,17 @@ cc_binary( "//test/util:test_util", ], ) + +cc_binary( + name = "unlink_test", + testonly = 1, + srcs = ["unlink_test.cc"], + deps = [ + gtest, + ":fuse_base", + "//test/util:fuse_util", + "//test/util:temp_umask", + "//test/util:test_main", + "//test/util:test_util", + ], +) diff --git a/test/fuse/linux/unlink_test.cc b/test/fuse/linux/unlink_test.cc new file mode 100644 index 000000000..5702e9b32 --- /dev/null +++ b/test/fuse/linux/unlink_test.cc @@ -0,0 +1,83 @@ +// 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 +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/fuse/linux/fuse_base.h" +#include "test/util/fuse_util.h" +#include "test/util/test_util.h" + +namespace gvisor { +namespace testing { + +namespace { + +class UnlinkTest : public FuseTest { + protected: + const std::string test_file_ = "test_file"; +}; + +TEST_F(UnlinkTest, RegularFile) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + SetServerInodeLookup(test_file_, S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header), + }; + auto iov_out = FuseGenerateIovecs(out_header); + SetServerResponse(FUSE_UNLINK, iov_out); + + ASSERT_THAT(unlink(test_file_path.c_str()), SyscallSucceeds()); + struct fuse_in_header in_header; + std::vector unlinked_file(test_file_.length() + 1); + auto iov_in = FuseGenerateIovecs(in_header, unlinked_file); + GetServerActualRequest(iov_in); + + EXPECT_EQ(in_header.len, sizeof(in_header) + test_file_.length() + 1); + EXPECT_EQ(in_header.opcode, FUSE_UNLINK); + EXPECT_EQ(std::string(unlinked_file.data()), test_file_); +} + +TEST_F(UnlinkTest, NoFile) { + const std::string test_file_path = + JoinPath(mount_point_.path().c_str(), test_file_); + SetServerInodeLookup(test_file_, S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO); + + struct fuse_out_header out_header = { + .len = sizeof(struct fuse_out_header), + .error = -ENOENT, + }; + auto iov_out = FuseGenerateIovecs(out_header); + SetServerResponse(FUSE_UNLINK, iov_out); + + ASSERT_THAT(unlink(test_file_path.c_str()), SyscallFailsWithErrno(ENOENT)); + SkipServerActualRequest(); +} + +} // namespace + +} // namespace testing +} // namespace gvisor -- cgit v1.2.3 From 8ab32686e274cbebbbb7a8794af40149d5ead7f2 Mon Sep 17 00:00:00 2001 From: Boyuan He Date: Fri, 11 Sep 2020 23:28:13 +0000 Subject: fuse_open: add padding to open out request --- pkg/abi/linux/fuse.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index fdd22a13d..c2357a2ee 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -156,9 +156,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, @@ -402,6 +402,8 @@ type FUSEOpenOut struct { // OpenFlag for the opened file. OpenFlag uint32 + + _ uint32 } // FUSE_READ flags, consistent with the ones in include/uapi/linux/fuse.h. -- cgit v1.2.3 From c6e749fb32efc38fa51e2997f622e2279b1b935e Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Mon, 14 Sep 2020 10:56:25 -0700 Subject: fs/fuse: Move the 'marshal' and 'primitive' packages to the 'pkg' directory. --- pkg/abi/linux/fuse.go | 4 ++-- pkg/sentry/fsimpl/fuse/connection.go | 1 - pkg/sentry/fsimpl/fuse/fusefs.go | 2 +- pkg/sentry/fsimpl/fuse/request_response.go | 2 +- pkg/sentry/fsimpl/fuse/utils_test.go | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index c2357a2ee..d91c97a64 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -15,8 +15,8 @@ package linux import ( - "gvisor.dev/gvisor/tools/go_marshal/marshal" - "gvisor.dev/gvisor/tools/go_marshal/primitive" + "gvisor.dev/gvisor/pkg/marshal" + "gvisor.dev/gvisor/pkg/marshal/primitive" ) // +marshal diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index fedf4d067..a7402c149 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -20,7 +20,6 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/log" - "gvisor.dev/gvisor/pkg/marshal" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/syserror" diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 402dabe5a..86922a96a 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -24,13 +24,13 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/marshal" "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/vfs" "gvisor.dev/gvisor/pkg/syserror" "gvisor.dev/gvisor/pkg/waiter" - "gvisor.dev/gvisor/tools/go_marshal/marshal" ) // Name is the default filesystem name. diff --git a/pkg/sentry/fsimpl/fuse/request_response.go b/pkg/sentry/fsimpl/fuse/request_response.go index 70db50f38..7fa00569b 100644 --- a/pkg/sentry/fsimpl/fuse/request_response.go +++ b/pkg/sentry/fsimpl/fuse/request_response.go @@ -19,10 +19,10 @@ import ( "syscall" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/marshal" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/usermem" - "gvisor.dev/gvisor/tools/go_marshal/marshal" ) // fuseInitRes is a variable-length wrapper of linux.FUSEInitOut. The FUSE diff --git a/pkg/sentry/fsimpl/fuse/utils_test.go b/pkg/sentry/fsimpl/fuse/utils_test.go index 436322830..ec2d2c7bc 100644 --- a/pkg/sentry/fsimpl/fuse/utils_test.go +++ b/pkg/sentry/fsimpl/fuse/utils_test.go @@ -18,12 +18,12 @@ import ( "testing" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/marshal" "gvisor.dev/gvisor/pkg/sentry/fsimpl/testutil" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/usermem" - "gvisor.dev/gvisor/tools/go_marshal/marshal" ) func setup(t *testing.T) *testutil.System { -- cgit v1.2.3