From 7fac7e32f3a866134bcee499dfc64459946dfe9d Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Thu, 18 Mar 2021 12:14:01 -0700 Subject: Translate syserror when validating partial IO errors syserror allows packages to register translators for errors. These translators should be called prior to checking if the error is valid, otherwise it may not account for possible errors that can be returned from different packages, e.g. safecopy.BusError => syserror.EFAULT. Second attempt, it passes tests now :-) PiperOrigin-RevId: 363714508 --- test/syscalls/linux/BUILD | 3 ++ test/syscalls/linux/read.cc | 40 +++++++++++++++++++++++ test/syscalls/linux/write.cc | 78 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+) (limited to 'test/syscalls') diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 037181f7e..043ada583 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -1922,7 +1922,9 @@ cc_binary( linkstatic = 1, deps = [ "//test/util:file_descriptor", + "@com_google_absl//absl/base:core_headers", gtest, + "//test/util:cleanup", "//test/util:temp_path", "//test/util:test_main", "//test/util:test_util", @@ -3991,6 +3993,7 @@ cc_binary( linkstatic = 1, deps = [ "//test/util:cleanup", + "@com_google_absl//absl/base:core_headers", gtest, "//test/util:temp_path", "//test/util:test_main", diff --git a/test/syscalls/linux/read.cc b/test/syscalls/linux/read.cc index 98d5e432d..087262535 100644 --- a/test/syscalls/linux/read.cc +++ b/test/syscalls/linux/read.cc @@ -13,11 +13,14 @@ // limitations under the License. #include +#include #include #include #include "gtest/gtest.h" +#include "absl/base/macros.h" +#include "test/util/cleanup.h" #include "test/util/file_descriptor.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" @@ -121,6 +124,43 @@ TEST_F(ReadTest, ReadWithOpath) { EXPECT_THAT(ReadFd(fd.get(), buf.data(), 1), SyscallFailsWithErrno(EBADF)); } +// Test that partial writes that hit SIGSEGV are correctly handled and return +// partial write. +TEST_F(ReadTest, PartialReadSIGSEGV) { + // Allocate 2 pages and remove permission from the second. + const size_t size = 2 * kPageSize; + void* addr = + mmap(0, size, PROT_WRITE | PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); + ASSERT_NE(addr, MAP_FAILED); + auto cleanup = Cleanup( + [addr, size] { EXPECT_THAT(munmap(addr, size), SyscallSucceeds()); }); + + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(name_.c_str(), O_RDWR, 0666)); + for (size_t i = 0; i < 2; i++) { + EXPECT_THAT(pwrite(fd.get(), addr, size, 0), + SyscallSucceedsWithValue(size)); + } + + void* badAddr = reinterpret_cast(addr) + kPageSize; + ASSERT_THAT(mprotect(badAddr, kPageSize, PROT_NONE), SyscallSucceeds()); + + // Attempt to read to both pages. Create a non-contiguous iovec pair to + // ensure operation is done in 2 steps. + struct iovec iov[] = { + { + .iov_base = addr, + .iov_len = kPageSize, + }, + { + .iov_base = addr, + .iov_len = size, + }, + }; + EXPECT_THAT(preadv(fd.get(), iov, ABSL_ARRAYSIZE(iov), 0), + SyscallSucceedsWithValue(size)); +} + } // namespace } // namespace testing diff --git a/test/syscalls/linux/write.cc b/test/syscalls/linux/write.cc index 740992d0a..3373ba72b 100644 --- a/test/syscalls/linux/write.cc +++ b/test/syscalls/linux/write.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -23,6 +24,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "absl/base/macros.h" #include "test/util/cleanup.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" @@ -256,6 +258,82 @@ TEST_F(WriteTest, PwriteWithOpath) { SyscallFailsWithErrno(EBADF)); } +// Test that partial writes that hit SIGSEGV are correctly handled and return +// partial write. +TEST_F(WriteTest, PartialWriteSIGSEGV) { + // Allocate 2 pages and remove permission from the second. + const size_t size = 2 * kPageSize; + void* addr = mmap(0, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); + ASSERT_NE(addr, MAP_FAILED); + auto cleanup = Cleanup( + [addr, size] { EXPECT_THAT(munmap(addr, size), SyscallSucceeds()); }); + + void* badAddr = reinterpret_cast(addr) + kPageSize; + ASSERT_THAT(mprotect(badAddr, kPageSize, PROT_NONE), SyscallSucceeds()); + + TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_WRONLY)); + + // Attempt to write both pages to the file. Create a non-contiguous iovec pair + // to ensure operation is done in 2 steps. + struct iovec iov[] = { + { + .iov_base = addr, + .iov_len = kPageSize, + }, + { + .iov_base = addr, + .iov_len = size, + }, + }; + // Write should succeed for the first iovec and half of the second (=2 pages). + EXPECT_THAT(pwritev(fd.get(), iov, ABSL_ARRAYSIZE(iov), 0), + SyscallSucceedsWithValue(2 * kPageSize)); +} + +// Test that partial writes that hit SIGBUS are correctly handled and return +// partial write. +TEST_F(WriteTest, PartialWriteSIGBUS) { + SKIP_IF(getenv("GVISOR_GOFER_UNCACHED")); // Can't mmap from uncached files. + + TempPath mapfile = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + FileDescriptor fd_map = + ASSERT_NO_ERRNO_AND_VALUE(Open(mapfile.path().c_str(), O_RDWR)); + + // Let the first page be read to force a partial write. + ASSERT_THAT(ftruncate(fd_map.get(), kPageSize), SyscallSucceeds()); + + // Map 2 pages, one of which is not allocated in the backing file. Reading + // from it will trigger a SIGBUS. + const size_t size = 2 * kPageSize; + void* addr = + mmap(NULL, size, PROT_READ, MAP_FILE | MAP_PRIVATE, fd_map.get(), 0); + ASSERT_NE(addr, MAP_FAILED); + auto cleanup = Cleanup( + [addr, size] { EXPECT_THAT(munmap(addr, size), SyscallSucceeds()); }); + + TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_WRONLY)); + + // Attempt to write both pages to the file. Create a non-contiguous iovec pair + // to ensure operation is done in 2 steps. + struct iovec iov[] = { + { + .iov_base = addr, + .iov_len = kPageSize, + }, + { + .iov_base = addr, + .iov_len = size, + }, + }; + // Write should succeed for the first iovec and half of the second (=2 pages). + ASSERT_THAT(pwritev(fd.get(), iov, ABSL_ARRAYSIZE(iov), 0), + SyscallSucceedsWithValue(2 * kPageSize)); +} + } // namespace } // namespace testing -- cgit v1.2.3