From 15ff2893d9bf896cc33e880983cb800cd71c946b Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Mon, 17 Aug 2020 15:33:19 -0700 Subject: Extend integration test to test sequence of FUSE operation Original FUSE integration test has limited capabilities. To test more situations, the new integration test framework introduces a protocol to communicate between testing thread and the FUSE server. In summary, this change includes: 1. Remove CompareResult() and break SetExpected() into SetServerResponse() and GetServerActualRequest(). We no longer set up an expected request because we want to retrieve the actual FUSE request made to the FUSE server and check in the testing thread. 2. Declare a serial buffer data structure to save the received requests and expected responses sequentially. The data structure contains a cursor to indicate the progress of accessing. This change makes sequential SetServerResponse() and GetServerActualRequest() possible. 3. Replace 2 single directional pipes with 1 bi-directional socketpair. A protocol which starts with FuseTestCmd is used between the testing thread and the FUSE server to provide various functionality. Fixes #3405 --- test/fuse/linux/fuse_base.h | 172 +++++++++++++++++++++++++++++++++----------- 1 file changed, 131 insertions(+), 41 deletions(-) (limited to 'test/fuse/linux/fuse_base.h') diff --git a/test/fuse/linux/fuse_base.h b/test/fuse/linux/fuse_base.h index 3a2f255a9..b610d0f54 100644 --- a/test/fuse/linux/fuse_base.h +++ b/test/fuse/linux/fuse_base.h @@ -16,8 +16,10 @@ #define GVISOR_TEST_FUSE_FUSE_BASE_H_ #include +#include #include +#include #include #include "gtest/gtest.h" @@ -29,68 +31,156 @@ namespace testing { constexpr char kMountOpts[] = "rootmode=755,user_id=0,group_id=0"; -class FuseTest : public ::testing::Test { +// Internal commands used to communicate between testing thread and the FUSE +// server. See test/fuse/README.md for further detail. +enum class FuseTestCmd { + kSetResponse = 0, + kGetRequest, +}; + +// Holds the information of a memory block in a serial buffer. +struct FuseMemBlock { + uint32_t opcode; + size_t offset; + size_t len; +}; + +// A wrapper of a simple serial buffer that can be used with read(2) and +// write(2). Contains a cursor to indicate accessing. This class is not thread- +// safe and can only be used in single-thread version. +class FuseMemBuffer { public: - FuseTest() { - buf_.resize(FUSE_MIN_READ_BUFFER); - mem_in_.resize(FUSE_MIN_READ_BUFFER); - mem_out_.resize(FUSE_MIN_READ_BUFFER); + FuseMemBuffer() : cursor_(0) { + // To read from /dev/fuse, a buffer needs at least FUSE_MIN_READ_BUFFER + // bytes to avoid EINVAL. FuseMemBuffer holds memory that can accommodate + // a sequence of FUSE request/response, so it is initiated with double + // minimal requirement. + mem_.resize(FUSE_MIN_READ_BUFFER * 2); } + + // Returns whether there is no memory block. + bool Empty() { return blocks_.empty(); } + + // Returns if there is no more remaining memory blocks. + bool End() { return cursor_ == blocks_.size(); } + + // Returns how many bytes that have been received. + size_t UsedBytes() { + return Empty() ? 0 : blocks_.back().offset + blocks_.back().len; + } + + // Returns the available bytes remains in the serial buffer. + size_t AvailBytes() { return mem_.size() - UsedBytes(); } + + // Appends a memory block information that starts at the tail of the serial + // buffer. /dev/fuse requires at least FUSE_MIN_READ_BUFFER bytes to read, or + // it will issue EINVAL. If it is not enough, just double the buffer length. + void AddMemBlock(uint32_t opcode, void* data, size_t len) { + if (AvailBytes() < FUSE_MIN_READ_BUFFER) { + mem_.resize(mem_.size() << 1); + } + size_t offset = UsedBytes(); + memcpy(mem_.data() + offset, data, len); + blocks_.push_back(FuseMemBlock{opcode, offset, len}); + } + + // Returns the memory address at a specific offset. Used with read(2) or + // write(2). + char* DataAtOffset(size_t offset) { return mem_.data() + offset; } + + // Returns current memory block pointed by the cursor and increase by 1. + FuseMemBlock Next() { + if (End()) { + std::cerr << "Buffer is already exhausted." << std::endl; + return FuseMemBlock{}; + } + return blocks_[cursor_++]; + } + + // Returns the number of the blocks that has not been requested. + size_t RemainingBlocks() { return blocks_.size() - cursor_; } + + private: + size_t cursor_; + std::vector blocks_; + std::vector mem_; +}; + +// FuseTest base class is useful in FUSE integration test. Inherit this class +// to automatically set up a fake FUSE server and use the member functions +// to manipulate with it. Refer to test/fuse/README.md for detailed explanation. +class FuseTest : public ::testing::Test { + public: void SetUp() override; void TearDown() override; - // CompareRequest is used by the FUSE server and should be implemented to - // compare different FUSE operations. It compares the actual FUSE input - // request with the expected one set by `SetExpected()`. - virtual bool CompareRequest(void* expected_mem, size_t expected_len, - void* real_mem, size_t real_len); - - // SetExpected is called by the testing main thread. Writes a request- - // response pair into FUSE server's member variables via pipe. - void SetExpected(struct iovec* iov_in, int iov_in_cnt, struct iovec* iov_out, - int iov_out_cnt); + // Called by the testing thread to set up a fake response for an expected + // opcode via socket. This can be used multiple times to define a sequence of + // expected FUSE reactions. + void SetServerResponse(uint32_t opcode, std::vector& iovecs); - // WaitCompleted waits for FUSE server to complete its processing. It - // complains if the FUSE server responds failure during tests. - void WaitCompleted(); + // 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 + // data from server. + void GetServerActualRequest(std::vector& iovecs); protected: TempPath mount_point_; private: + // Opens /dev/fuse and inherit the file descriptor for the FUSE server. void MountFuse(); + + // Unmounts the mountpoint of the FUSE server. void UnmountFuse(); - // ConsumeFuseInit is only used during FUSE server setup. - PosixError ConsumeFuseInit(); + // Creates a socketpair for communication and forks FUSE server. + void SetUpFuseServer(); - // ReceiveExpected is the FUSE server side's corresponding code of - // `SetExpected()`. Save the request-response pair into its memory. - void ReceiveExpected(); + // Waits for FUSE server to complete its processing. Complains if the FUSE + // server responds any failure during tests. + void WaitServerComplete(); - // MarkDone is used by the FUSE server to tell testing main if it's OK to - // proceed next command. - void MarkDone(bool success); + // The FUSE server stays here and waits next command or FUSE request until it + // is terminated. + void ServerFuseLoop(); - // FuseLoop is where the FUSE server stay until it is terminated. - void FuseLoop(); + // Used by the FUSE server to tell testing thread if it is OK to proceed next + // command. Will be issued after processing each FuseTestCmd. + void ServerCompleteWith(bool success); - // SetUpFuseServer creates 2 pipes for communication and forks FUSE server. - void SetUpFuseServer(); + // Consumes the first FUSE request when mounting FUSE. Replies with a + // response with empty payload. + PosixError ServerConsumeFuseInit(); + + // A command switch that dispatch different FuseTestCmd to its handler. + void ServerHandleCommand(); + + // The FUSE server side's corresponding code of `SetServerResponse()`. + // Handles `kSetResponse` command. Saves the fake response into its output + // memory queue. + void ServerReceiveResponse(); + + // The FUSE server side's corresponding code of `GetServerActualRequest()`. + // Handles `kGetRequest` command. Sends the next received request pointed by + // the cursor. + void ServerSendReceivedRequest(); - // GetPayloadSize is a helper function to get the number of bytes of a - // specific FUSE operation struct. - size_t GetPayloadSize(uint32_t opcode, bool in); + // Handles FUSE request sent to /dev/fuse by its saved responses. + void ServerProcessFuseRequest(); + + // Responds to FUSE request with a saved data. + void ServerRespondFuseSuccess(FuseMemBuffer& mem_buf, + const FuseMemBlock& block, uint64_t unique); + + // Responds an error header to /dev/fuse when bad thing happens. + void ServerRespondFuseError(uint64_t unique); int dev_fd_; - int set_expected_[2]; - int done_[2]; - - std::vector buf_; - std::vector mem_in_; - std::vector mem_out_; - ssize_t len_in_; - ssize_t len_out_; + int sock_[2]; + + FuseMemBuffer requests_; + FuseMemBuffer responses_; }; } // namespace testing -- cgit v1.2.3 From a289c3862653cded271408b31a9e704be615503a Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Fri, 14 Aug 2020 09:54:35 -0700 Subject: Add functions in FUSE integration test to get metrics from FUSE server This commit adds 3 utility functions to ensure all received requests and preset responses are consumed. 1. Get number of unconsumed requests (received by the FUSE server but not consumed by the testing thread). 2. Get number of unsent responses (set by the testing thread but not processed by the FUSE server). 3. Get total bytes of the received requests (to ensure some operations don't trigger FUSE requests). Fixes #3607 --- test/fuse/README.md | 27 +++++++++++++++------ test/fuse/linux/fuse_base.cc | 58 +++++++++++++++++++++++++++++++++++++++----- test/fuse/linux/fuse_base.h | 24 ++++++++++++++++++ 3 files changed, 96 insertions(+), 13 deletions(-) (limited to 'test/fuse/linux/fuse_base.h') diff --git a/test/fuse/README.md b/test/fuse/README.md index c5909a166..7a1839714 100644 --- a/test/fuse/README.md +++ b/test/fuse/README.md @@ -86,6 +86,20 @@ complete a command and when the server awaits the next instruction. | =[Test actual request] | | | | >TearDown() | + | ... | + | >GetServerNumUnsentResponses() | + | [write data to socket] | + | [wait server complete] | + | | [socket event arrive] + | | >ServerHandleCommand() + | | >ServerSendData() + | | [write data to socket] + | | (1)); + ASSERT_EQ(success, 1); } // Sends the `kGetRequest` command to the FUSE server, then reads the next @@ -83,6 +87,35 @@ void FuseTest::GetServerActualRequest(std::vector& iovecs) { WaitServerComplete(); } +// Sends a FuseTestCmd command to the FUSE server, reads from the socket, and +// returns the corresponding data. +uint32_t FuseTest::GetServerData(uint32_t cmd) { + uint32_t data; + EXPECT_THAT(RetryEINTR(write)(sock_[0], &cmd, sizeof(cmd)), + SyscallSucceedsWithValue(sizeof(cmd))); + + EXPECT_THAT(RetryEINTR(read)(sock_[0], &data, sizeof(data)), + SyscallSucceedsWithValue(sizeof(data))); + + WaitServerComplete(); + return data; +} + +uint32_t FuseTest::GetServerNumUnconsumedRequests() { + return GetServerData( + static_cast(FuseTestCmd::kGetNumUnconsumedRequests)); +} + +uint32_t FuseTest::GetServerNumUnsentResponses() { + return GetServerData( + static_cast(FuseTestCmd::kGetNumUnsentResponses)); +} + +uint32_t FuseTest::GetServerTotalReceivedBytes() { + return GetServerData( + static_cast(FuseTestCmd::kGetTotalReceivedBytes)); +} + void FuseTest::MountFuse() { EXPECT_THAT(dev_fd_ = open("/dev/fuse", O_RDWR), SyscallSucceeds()); @@ -141,9 +174,8 @@ void FuseTest::ServerReceiveResponse() { // Writes 1 byte of success indicator through socket. void FuseTest::ServerCompleteWith(bool success) { - char data = static_cast(success); - EXPECT_THAT(RetryEINTR(write)(sock_[1], &data, sizeof(data)), - SyscallSucceedsWithValue(sizeof(data))); + uint32_t data = success ? 1 : 0; + ServerSendData(data); } // ServerFuseLoop is the implementation of the fake FUSE server. Monitors 2 @@ -205,6 +237,11 @@ void FuseTest::SetUpFuseServer() { _exit(0); } +void FuseTest::ServerSendData(uint32_t data) { + EXPECT_THAT(RetryEINTR(write)(sock_[1], &data, sizeof(data)), + SyscallSucceedsWithValue(sizeof(data))); +} + // Reads FuseTestCmd sent from testing thread and routes to correct handler. // Since each command should be a blocking operation, a `ServerCompleteWith()` // is required after the switch keyword. @@ -220,6 +257,15 @@ void FuseTest::ServerHandleCommand() { case FuseTestCmd::kGetRequest: ServerSendReceivedRequest(); break; + case FuseTestCmd::kGetTotalReceivedBytes: + ServerSendData(static_cast(requests_.UsedBytes())); + break; + case FuseTestCmd::kGetNumUnconsumedRequests: + ServerSendData(static_cast(requests_.RemainingBlocks())); + break; + case FuseTestCmd::kGetNumUnsentResponses: + ServerSendData(static_cast(responses_.RemainingBlocks())); + break; default: FAIL() << "Unknown FuseTestCmd " << cmd; break; diff --git a/test/fuse/linux/fuse_base.h b/test/fuse/linux/fuse_base.h index b610d0f54..3f2522977 100644 --- a/test/fuse/linux/fuse_base.h +++ b/test/fuse/linux/fuse_base.h @@ -36,6 +36,9 @@ constexpr char kMountOpts[] = "rootmode=755,user_id=0,group_id=0"; enum class FuseTestCmd { kSetResponse = 0, kGetRequest, + kGetNumUnconsumedRequests, + kGetNumUnsentResponses, + kGetTotalReceivedBytes, }; // Holds the information of a memory block in a serial buffer. @@ -124,6 +127,21 @@ class FuseTest : public ::testing::Test { // data from server. void GetServerActualRequest(std::vector& iovecs); + // Called by the testing thread to query the number of unconsumed requests in + // the requests_ serial buffer of the FUSE server. TearDown() ensures all + // FUSE requests received by the FUSE server were consumed by the testing + // thread. + uint32_t GetServerNumUnconsumedRequests(); + + // Called by the testing thread to query the number of unsent responses in + // the responses_ serial buffer of the FUSE server. TearDown() ensures all + // preset FUSE responses were sent out by the FUSE server. + uint32_t GetServerNumUnsentResponses(); + + // Called by the testing thread to ask the FUSE server for its total received + // bytes from /dev/fuse. + uint32_t GetServerTotalReceivedBytes(); + protected: TempPath mount_point_; @@ -137,6 +155,9 @@ class FuseTest : public ::testing::Test { // Creates a socketpair for communication and forks FUSE server. void SetUpFuseServer(); + // Sends a FuseTestCmd and gets a uint32_t data from the FUSE server. + inline uint32_t GetServerData(uint32_t cmd); + // Waits for FUSE server to complete its processing. Complains if the FUSE // server responds any failure during tests. void WaitServerComplete(); @@ -166,6 +187,9 @@ class FuseTest : public ::testing::Test { // the cursor. void ServerSendReceivedRequest(); + // Sends a uint32_t data via socket. + inline void ServerSendData(uint32_t data); + // Handles FUSE request sent to /dev/fuse by its saved responses. void ServerProcessFuseRequest(); -- cgit v1.2.3 From 717b661c457cc3a125fcdfd133b633ca48545541 Mon Sep 17 00:00:00 2001 From: Craig Chi Date: Mon, 17 Aug 2020 10:05:10 -0700 Subject: Add function to create a fake inode in FUSE integration test Adds a function for the testing thread to set up a fake inode with a specific path under mount point. After this function is called, each subsequent FUSE_LOOKUP request with the same path will be served with the fixed stub response. Fixes #3539 --- test/fuse/linux/fuse_base.cc | 91 ++++++++++++++++++++++++++++++++++++++++++++ test/fuse/linux/fuse_base.h | 25 ++++++++++++ 2 files changed, 116 insertions(+) (limited to 'test/fuse/linux/fuse_base.h') diff --git a/test/fuse/linux/fuse_base.cc b/test/fuse/linux/fuse_base.cc index c354e1dcb..a9fe1044e 100644 --- a/test/fuse/linux/fuse_base.cc +++ b/test/fuse/linux/fuse_base.cc @@ -117,6 +117,23 @@ uint32_t FuseTest::GetServerTotalReceivedBytes() { static_cast(FuseTestCmd::kGetTotalReceivedBytes)); } +// 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) { + uint32_t cmd = static_cast(FuseTestCmd::kSetInodeLookup); + EXPECT_THAT(RetryEINTR(write)(sock_[0], &cmd, sizeof(cmd)), + SyscallSucceedsWithValue(sizeof(cmd))); + + EXPECT_THAT(RetryEINTR(write)(sock_[0], &mode, sizeof(mode)), + SyscallSucceedsWithValue(sizeof(mode))); + + // Pad 1 byte for null-terminate c-string. + EXPECT_THAT(RetryEINTR(write)(sock_[0], path.c_str(), path.size() + 1), + SyscallSucceedsWithValue(path.size() + 1)); + + WaitServerComplete(); +} + void FuseTest::MountFuse() { EXPECT_THAT(dev_fd_ = open("/dev/fuse", O_RDWR), SyscallSucceeds()); @@ -252,6 +269,9 @@ void FuseTest::ServerHandleCommand() { case FuseTestCmd::kSetResponse: ServerReceiveResponse(); break; + case FuseTestCmd::kSetInodeLookup: + ServerReceiveInodeLookup(); + break; case FuseTestCmd::kGetRequest: ServerSendReceivedRequest(); break; @@ -272,6 +292,64 @@ void FuseTest::ServerHandleCommand() { ServerCompleteWith(!HasFailure()); } +// Reads the expected file mode and the path of one file. Crafts a basic +// `fuse_entry_out` memory block and inserts into a map for future use. +// The FUSE server will always return this response if a FUSE_LOOKUP +// request with this specific path comes in. +void FuseTest::ServerReceiveInodeLookup() { + mode_t mode; + 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], buf.data(), buf.size()), + SyscallSucceeds()); + + std::string path(buf.data()); + + uint32_t out_len = + sizeof(struct fuse_out_header) + sizeof(struct fuse_entry_out); + struct fuse_out_header out_header = { + .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, + }, + }; + // Since this is only used in test, nodeid_ is simply increased by 1 to + // comply with the unqiueness of different path. + ++nodeid_; + + 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); + lookup_map_[path] = lookups_.Next(); +} + // Sends the received request pointed by current cursor and advances cursor. void FuseTest::ServerSendReceivedRequest() { if (requests_.End()) { @@ -297,6 +375,19 @@ void FuseTest::ServerProcessFuseRequest() { EXPECT_THAT(len = RetryEINTR(read)(dev_fd_, buf.data(), buf.size()), SyscallSucceeds()); fuse_in_header* in_header = reinterpret_cast(buf.data()); + + // Check if this is a preset FUSE_LOOKUP path. + if (in_header->opcode == FUSE_LOOKUP) { + std::string path(buf.data() + sizeof(struct fuse_in_header)); + auto it = lookup_map_.find(path); + if (it != lookup_map_.end()) { + // Matches a preset path. Reply with fake data and skip saving the + // request. + ServerRespondFuseSuccess(lookups_, it->second, in_header->unique); + return; + } + } + requests_.AddMemBlock(in_header->opcode, buf.data(), len); // Check if there is a corresponding response. diff --git a/test/fuse/linux/fuse_base.h b/test/fuse/linux/fuse_base.h index 3f2522977..a21b4bb8d 100644 --- a/test/fuse/linux/fuse_base.h +++ b/test/fuse/linux/fuse_base.h @@ -17,9 +17,11 @@ #include #include +#include #include #include +#include #include #include "gtest/gtest.h" @@ -35,6 +37,7 @@ constexpr char kMountOpts[] = "rootmode=755,user_id=0,group_id=0"; // server. See test/fuse/README.md for further detail. enum class FuseTestCmd { kSetResponse = 0, + kSetInodeLookup, kGetRequest, kGetNumUnconsumedRequests, kGetNumUnsentResponses, @@ -114,6 +117,9 @@ class FuseMemBuffer { // to manipulate with it. Refer to test/fuse/README.md for detailed explanation. class FuseTest : public ::testing::Test { public: + // nodeid_ is the ID of a fake inode. We starts from 2 since 1 is occupied by + // the mount point. + FuseTest() : nodeid_(2) {} void SetUp() override; void TearDown() override; @@ -122,6 +128,16 @@ class FuseTest : public ::testing::Test { // expected FUSE reactions. void SetServerResponse(uint32_t opcode, std::vector& iovecs); + // Called by the testing thread to install a fake path under the mount point. + // e.g. a file under /mnt/dir/file and moint point is /mnt, then it will look + // up "dir/file" in this case. + // + // It sets a fixed response to the FUSE_LOOKUP requests issued with this + // 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); + // 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 // data from server. @@ -182,6 +198,11 @@ class FuseTest : public ::testing::Test { // memory queue. void ServerReceiveResponse(); + // The FUSE server side's corresponding code of `SetServerInodeLookup()`. + // Handles `kSetInodeLookup` command. Receives an expected file mode and + // file path under the mount point. + void ServerReceiveInodeLookup(); + // The FUSE server side's corresponding code of `GetServerActualRequest()`. // Handles `kGetRequest` command. Sends the next received request pointed by // the cursor. @@ -203,8 +224,12 @@ class FuseTest : public ::testing::Test { int dev_fd_; int sock_[2]; + uint64_t nodeid_; + std::unordered_map lookup_map_; + FuseMemBuffer requests_; FuseMemBuffer responses_; + FuseMemBuffer lookups_; }; } // namespace testing -- 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 'test/fuse/linux/fuse_base.h') 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 'test/fuse/linux/fuse_base.h') 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 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 'test/fuse/linux/fuse_base.h') 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 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 'test/fuse/linux/fuse_base.h') 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