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/sentry/fsimpl/fuse/file.go | 96 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 pkg/sentry/fsimpl/fuse/file.go (limited to 'pkg/sentry/fsimpl/fuse/file.go') 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 +} -- 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/sentry/fsimpl/fuse/file.go') 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 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/sentry/fsimpl/fuse/file.go') 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 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/sentry/fsimpl/fuse/file.go') 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 4edc56d3e99b161f2f3311bc22a51012bf0a90ee Mon Sep 17 00:00:00 2001 From: Jinmou Li Date: Thu, 3 Sep 2020 19:22:24 +0000 Subject: Fix FUSE_RELEASE protocol reply processing This commit fixes the potential unexpected errors of original handling of FUSE_RELEASE responses while keep the same behavior (ignoring any reply). --- pkg/sentry/fsimpl/fuse/dev.go | 9 ++++++++- pkg/sentry/fsimpl/fuse/file.go | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 'pkg/sentry/fsimpl/fuse/file.go') diff --git a/pkg/sentry/fsimpl/fuse/dev.go b/pkg/sentry/fsimpl/fuse/dev.go index e7296c189..6f0b896cb 100644 --- a/pkg/sentry/fsimpl/fuse/dev.go +++ b/pkg/sentry/fsimpl/fuse/dev.go @@ -204,8 +204,11 @@ func (fd *DeviceFD) readLocked(ctx context.Context, dst usermem.IOSequence, opts // Fully done with this req, remove it from the queue. fd.queue.Remove(req) - if req.hdr.Opcode == linux.FUSE_RELEASE { + + // Remove noReply ones from map of requests expecting a reply. + if req.noReply { fd.numActiveRequests -= 1 + delete(fd.completions, req.hdr.Unique) } return int64(n), nil @@ -296,6 +299,10 @@ func (fd *DeviceFD) writeLocked(ctx context.Context, src usermem.IOSequence, opt fut, ok := fd.completions[hdr.Unique] if !ok { + if fut.hdr.Unique == linux.FUSE_RELEASE { + // Currently we simply discard the reply for FUSE_RELEASE. + return n + src.NumBytes(), nil + } // Server sent us a response for a request we never sent? return 0, syserror.EINVAL } diff --git a/pkg/sentry/fsimpl/fuse/file.go b/pkg/sentry/fsimpl/fuse/file.go index 15c0e3f41..b98145ba2 100644 --- a/pkg/sentry/fsimpl/fuse/file.go +++ b/pkg/sentry/fsimpl/fuse/file.go @@ -84,7 +84,12 @@ func (fd *fileDescription) Release(ctx context.Context) { } 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) + req, err := conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), fd.inode().NodeID, opcode, &in) + if err != nil { + // No way to invoke Call() with an errored request. + return + } + req.noReply = true conn.CallAsync(kernelTask, req) } -- cgit v1.2.3 From dd103527298375edd3e14600ac762c9a3103ac37 Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Wed, 9 Sep 2020 16:30:25 -0700 Subject: Unexport fusefs.inode.nodeID --- pkg/sentry/fsimpl/fuse/connection.go | 2 +- pkg/sentry/fsimpl/fuse/directory.go | 4 ++-- pkg/sentry/fsimpl/fuse/file.go | 7 ++++--- pkg/sentry/fsimpl/fuse/fusefs.go | 38 ++++++++++++++++++------------------ pkg/sentry/fsimpl/fuse/read_write.go | 4 ++-- 5 files changed, 28 insertions(+), 27 deletions(-) (limited to 'pkg/sentry/fsimpl/fuse/file.go') diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index 5960d9368..fedf4d067 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -47,7 +47,7 @@ const ( type connection struct { fd *DeviceFD - // mu protect access to struct memebers. + // mu protects access to struct memebers. mu sync.Mutex // attributeVersion is the version of connection's attributes. diff --git a/pkg/sentry/fsimpl/fuse/directory.go b/pkg/sentry/fsimpl/fuse/directory.go index a83357129..798c4a6f3 100644 --- a/pkg/sentry/fsimpl/fuse/directory.go +++ b/pkg/sentry/fsimpl/fuse/directory.go @@ -67,8 +67,8 @@ func (dir *directoryFD) IterDirents(ctx context.Context, callback vfs.IterDirent 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) + // 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 } diff --git a/pkg/sentry/fsimpl/fuse/file.go b/pkg/sentry/fsimpl/fuse/file.go index b98145ba2..991efcda4 100644 --- a/pkg/sentry/fsimpl/fuse/file.go +++ b/pkg/sentry/fsimpl/fuse/file.go @@ -84,7 +84,7 @@ func (fd *fileDescription) Release(ctx context.Context) { } kernelTask := kernel.TaskFromContext(ctx) // ignoring errors and FUSE server reply is analogous to Linux's behavior. - req, err := conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), fd.inode().NodeID, opcode, &in) + req, err := conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), fd.inode().nodeID, opcode, &in) if err != nil { // No way to invoke Call() with an errored request. return @@ -125,8 +125,9 @@ func (fd *fileDescription) Stat(ctx context.Context, opts vfs.StatOptions) (linu return inode.Stat(ctx, fs, opts) } -// SetStat implements FileDescriptionImpl.SetStat. +// SetStat implements vfs.FileDescriptionImpl.SetStat. func (fd *fileDescription) SetStat(ctx context.Context, opts vfs.SetStatOptions) error { + fs := fd.filesystem() creds := auth.CredentialsFromContext(ctx) - return fd.inode().setAttr(ctx, fd.inode().fs.VFSFilesystem(), creds, opts, true, fd.Fh) + return fd.inode().setAttr(ctx, fs, creds, opts, true, fd.Fh) } diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 8e749bdad..402dabe5a 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -254,7 +254,7 @@ type inode struct { // metaDataMu protects the metadata of this inode. metadataMu sync.Mutex - NodeID uint64 + nodeID uint64 locks vfs.FileLocks @@ -279,13 +279,13 @@ func (fs *filesystem) newRootInode(creds *auth.Credentials, mode linux.FileMode) i.InodeAttrs.Init(creds, linux.UNNAMED_MAJOR, fs.devMinor, 1, linux.ModeDirectory|0755) i.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) i.dentry.Init(i) - i.NodeID = 1 + i.nodeID = 1 return &i.dentry } func (fs *filesystem) newInode(nodeID uint64, attr linux.FUSEAttr) *kernfs.Dentry { - i := &inode{fs: fs, NodeID: nodeID} + 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) @@ -342,7 +342,7 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, vfsd *vfs.Dentr in.Flags &= ^uint32(linux.O_TRUNC) } - req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.NodeID, opcode, &in) + req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.nodeID, opcode, &in) if err != nil { return nil, err } @@ -405,13 +405,13 @@ func (i *inode) Lookup(ctx context.Context, name string) (*vfs.Dentry, error) { return i.newEntry(ctx, name, 0, linux.FUSE_LOOKUP, &in) } -// IterDirents implements Inode.IterDirents. -func (inode) IterDirents(ctx context.Context, callback vfs.IterDirentsCallback, offset, relOffset int64) (int64, error) { +// IterDirents implements kernfs.Inode.IterDirents. +func (*inode) IterDirents(ctx context.Context, callback vfs.IterDirentsCallback, offset, relOffset int64) (int64, error) { return offset, nil } -// Valid implements Inode.Valid. -func (inode) Valid(ctx context.Context) bool { +// Valid implements kernfs.Inode.Valid. +func (*inode) Valid(ctx context.Context) bool { return true } @@ -419,7 +419,7 @@ func (inode) Valid(ctx context.Context) bool { 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) + log.Warningf("fusefs.Inode.NewFile: couldn't get kernel task from context", i.nodeID) return nil, syserror.EINVAL } in := linux.FUSECreateIn{ @@ -459,11 +459,11 @@ func (i *inode) NewSymlink(ctx context.Context, name, target string) (*vfs.Dentr 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) + 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) + req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.nodeID, linux.FUSE_UNLINK, &in) if err != nil { return err } @@ -496,7 +496,7 @@ func (i *inode) RmDir(ctx context.Context, name string, child *vfs.Dentry) error 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) + req, err := fusefs.conn.NewRequest(creds, uint32(task.ThreadID()), i.nodeID, linux.FUSE_RMDIR, &in) if err != nil { return err } @@ -522,10 +522,10 @@ func (i *inode) RmDir(ctx context.Context, name string, child *vfs.Dentry) error func (i *inode) newEntry(ctx context.Context, name string, fileType linux.FileMode, opcode linux.FUSEOpcode, payload marshal.Marshallable) (*vfs.Dentry, error) { kernelTask := kernel.TaskFromContext(ctx) if kernelTask == nil { - log.Warningf("fusefs.Inode.newEntry: couldn't get kernel task from context", i.NodeID) + log.Warningf("fusefs.Inode.newEntry: couldn't get kernel task from context", i.nodeID) return nil, syserror.EINVAL } - req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.NodeID, opcode, payload) + req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.nodeID, opcode, payload) if err != nil { return nil, err } @@ -552,7 +552,7 @@ func (i *inode) newEntry(ctx context.Context, name string, fileType linux.FileMo return child.VFSDentry(), nil } -// Getlink implements Inode.Getlink. +// Getlink implements kernfs.Inode.Getlink. func (i *inode) Getlink(ctx context.Context, mnt *vfs.Mount) (vfs.VirtualDentry, string, error) { path, err := i.Readlink(ctx, mnt) return vfs.VirtualDentry{}, path, err @@ -563,13 +563,13 @@ 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 == "" { + if len(i.link) == 0 { 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{}) + req, err := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.nodeID, linux.FUSE_READLINK, &linux.FUSEEmptyIn{}) if err != nil { return "", err } @@ -675,7 +675,7 @@ func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOp GetAttrFlags: flags, Fh: fh, } - req, err := i.fs.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.FUSEAttr{}, err } @@ -798,7 +798,7 @@ func (i *inode) setAttr(ctx context.Context, fs *vfs.Filesystem, creds *auth.Cre UID: opts.Stat.UID, GID: opts.Stat.GID, } - req, err := conn.NewRequest(creds, uint32(task.ThreadID()), i.NodeID, linux.FUSE_SETATTR, &in) + req, err := conn.NewRequest(creds, uint32(task.ThreadID()), i.nodeID, linux.FUSE_SETATTR, &in) if err != nil { return err } diff --git a/pkg/sentry/fsimpl/fuse/read_write.go b/pkg/sentry/fsimpl/fuse/read_write.go index 22a018e5e..625d1547f 100644 --- a/pkg/sentry/fsimpl/fuse/read_write.go +++ b/pkg/sentry/fsimpl/fuse/read_write.go @@ -79,7 +79,7 @@ func (fs *filesystem) ReadInPages(ctx context.Context, fd *regularFileFD, off ui 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) + 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 } @@ -203,7 +203,7 @@ func (fs *filesystem) Write(ctx context.Context, fd *regularFileFD, off uint64, 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) + req, err := fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(t.ThreadID()), fd.inode().nodeID, linux.FUSE_WRITE, &in) if err != nil { return 0, err } -- cgit v1.2.3 From 70cf503b4c3653df8253438a8873a5d3ebb688e3 Mon Sep 17 00:00:00 2001 From: Jinmou Li Date: Wed, 16 Sep 2020 17:39:55 +0000 Subject: fuse: fix FUSE_RELEASE reply handling fix #3963 --- pkg/sentry/fsimpl/fuse/connection.go | 2 +- pkg/sentry/fsimpl/fuse/dev.go | 4 ---- pkg/sentry/fsimpl/fuse/file.go | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) (limited to 'pkg/sentry/fsimpl/fuse/file.go') diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index a7402c149..dbc5e1954 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -110,7 +110,7 @@ type connection struct { // // We call the "background" requests in unix term as async requests. // The "async requests" in unix term is our async requests that expect a reply, - // i.e. `!requestOptions.noReply` + // i.e. `!request.noReply` // asyncMu protects the async request fields. asyncMu sync.Mutex diff --git a/pkg/sentry/fsimpl/fuse/dev.go b/pkg/sentry/fsimpl/fuse/dev.go index a19544fe9..5539466ff 100644 --- a/pkg/sentry/fsimpl/fuse/dev.go +++ b/pkg/sentry/fsimpl/fuse/dev.go @@ -309,10 +309,6 @@ func (fd *DeviceFD) writeLocked(ctx context.Context, src usermem.IOSequence, opt fut, ok := fd.completions[hdr.Unique] if !ok { - if fut.hdr.Unique == linux.FUSE_RELEASE { - // Currently we simply discard the reply for FUSE_RELEASE. - return n + src.NumBytes(), nil - } // Server sent us a response for a request we never sent, // or for which we already received a reply (e.g. aborted), an unlikely event. return 0, syserror.EINVAL diff --git a/pkg/sentry/fsimpl/fuse/file.go b/pkg/sentry/fsimpl/fuse/file.go index 991efcda4..83f2816b7 100644 --- a/pkg/sentry/fsimpl/fuse/file.go +++ b/pkg/sentry/fsimpl/fuse/file.go @@ -89,7 +89,7 @@ func (fd *fileDescription) Release(ctx context.Context) { // No way to invoke Call() with an errored request. return } - req.noReply = true + // The reply will be ignored since no callback is defined in asyncCallBack(). conn.CallAsync(kernelTask, req) } -- cgit v1.2.3