diff --git a/sandboxed_api/BUILD b/sandboxed_api/BUILD index dcc27a20..d920b2ee 100644 --- a/sandboxed_api/BUILD +++ b/sandboxed_api/BUILD @@ -94,6 +94,7 @@ cc_library( "//sandboxed_api/sandbox2", "//sandboxed_api/sandbox2:client", "//sandboxed_api/sandbox2:comms", + "//sandboxed_api/sandbox2:util", "//sandboxed_api/util:file_base", "//sandboxed_api/util:fileops", "//sandboxed_api/util:runfiles", @@ -167,6 +168,7 @@ cc_library( ":proto_arg_cc_proto", ":var_type", "//sandboxed_api/sandbox2:comms", + "//sandboxed_api/sandbox2:util", "//sandboxed_api/util:status", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/log", @@ -176,6 +178,7 @@ cc_library( "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/synchronization", + "@com_google_absl//absl/types:span", "@com_google_absl//absl/utility", ], ) diff --git a/sandboxed_api/CMakeLists.txt b/sandboxed_api/CMakeLists.txt index a45eac20..0ab952ec 100644 --- a/sandboxed_api/CMakeLists.txt +++ b/sandboxed_api/CMakeLists.txt @@ -150,6 +150,7 @@ add_library(sapi_vars ${SAPI_LIB_TYPE} add_library(sapi::vars ALIAS sapi_vars) target_link_libraries(sapi_vars PRIVATE absl::core_headers + absl::span absl::status absl::statusor absl::str_format @@ -157,6 +158,7 @@ target_link_libraries(sapi_vars absl::synchronization absl::utility sandbox2::comms + sandbox2::util sapi::base sapi::call sapi::lenval_core diff --git a/sandboxed_api/sandbox.cc b/sandboxed_api/sandbox.cc index e898ab1d..25c2794b 100644 --- a/sandboxed_api/sandbox.cc +++ b/sandboxed_api/sandbox.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -46,6 +47,7 @@ #include "sandboxed_api/sandbox2/policybuilder.h" #include "sandboxed_api/sandbox2/result.h" #include "sandboxed_api/sandbox2/sandbox2.h" +#include "sandboxed_api/sandbox2/util.h" #include "sandboxed_api/util/path.h" #include "sandboxed_api/util/runfiles.h" #include "sandboxed_api/util/status_macros.h" @@ -450,21 +452,11 @@ absl::StatusOr Sandbox::GetCString(const v::RemotePtr& str, absl::StrCat("Target string too large: ", len, " > ", max_length)); } std::string buffer(len, '\0'); - struct iovec local = { - .iov_base = &buffer[0], - .iov_len = len, - }; - struct iovec remote = { - .iov_base = str.GetValue(), - .iov_len = len, - }; - - ssize_t ret = process_vm_readv(pid_, &local, 1, &remote, 1, 0); - if (ret == -1) { - PLOG(WARNING) << "reading c-string failed: process_vm_readv(pid: " << pid_ - << " raddr: " << str.GetValue() << " size: " << len << ")"; - return absl::UnavailableError("process_vm_readv failed"); - } + SAPI_ASSIGN_OR_RETURN( + size_t ret, + sandbox2::util::ReadBytesFromPidInto( + pid_, reinterpret_cast(str.GetValue()), + absl::MakeSpan(reinterpret_cast(buffer.data()), len))); if (ret != len) { LOG(WARNING) << "partial read when reading c-string: process_vm_readv(pid: " << pid_ << " raddr: " << str.GetValue() << " size: " << len diff --git a/sandboxed_api/sandbox2/BUILD b/sandboxed_api/sandbox2/BUILD index 35939561..40788f85 100644 --- a/sandboxed_api/sandbox2/BUILD +++ b/sandboxed_api/sandbox2/BUILD @@ -801,6 +801,7 @@ cc_library( "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", + "@com_google_absl//absl/types:span", ], ) @@ -1034,8 +1035,10 @@ cc_test( "//sandboxed_api:testing", "//sandboxed_api/util:status_matchers", "@com_google_absl//absl/cleanup", + "@com_google_absl//absl/log:check", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", + "@com_google_absl//absl/types:span", "@com_google_googletest//:gtest_main", ], ) diff --git a/sandboxed_api/sandbox2/CMakeLists.txt b/sandboxed_api/sandbox2/CMakeLists.txt index 521233fb..afb949fa 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -700,7 +700,8 @@ target_link_libraries(sandbox2_util sapi::base sapi::raw_logging sapi::status - PUBLIC absl::status + PUBLIC absl::span + absl::status absl::statusor ) target_compile_options(sandbox2_util PRIVATE @@ -1116,7 +1117,9 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING) sandbox2::util absl::statusor absl::strings + absl::check absl::cleanup + absl::span sapi::status_matchers sapi::testing sapi::test_main diff --git a/sandboxed_api/sandbox2/util.cc b/sandboxed_api/sandbox2/util.cc index 52d37425..d83fb746 100644 --- a/sandboxed_api/sandbox2/util.cc +++ b/sandboxed_api/sandbox2/util.cc @@ -14,6 +14,7 @@ #include "sandboxed_api/sandbox2/util.h" +#include #include #include #include @@ -50,6 +51,7 @@ #include "absl/strings/str_replace.h" #include "absl/strings/str_split.h" #include "absl/strings/string_view.h" +#include "absl/types/span.h" #include "sandboxed_api/config.h" #include "sandboxed_api/util/file_helpers.h" #include "sandboxed_api/util/fileops.h" @@ -410,39 +412,178 @@ std::string GetPtraceEventName(int event) { } } -absl::StatusOr> ReadBytesFromPid(pid_t pid, uintptr_t ptr, - uint64_t size) { +namespace { + +// Transfer memory via process_vm_readv/process_vm_writev. +absl::StatusOr ProcessVmTransfer(bool is_read, pid_t pid, uintptr_t ptr, + absl::Span data) { static const uintptr_t page_size = getpagesize(); static const uintptr_t page_mask = page_size - 1; // Input sanity checks. - if (size == 0) { - return std::vector(); + if (data.empty()) { + return 0; } - // Allocate enough bytes to hold the entire size. - std::vector bytes(size, 0); - iovec local_iov[] = {{bytes.data(), bytes.size()}}; + iovec local_iov[] = {{data.data(), data.size()}}; // Stores all the necessary iovecs to move memory. std::vector remote_iov; // Each iovec should be contained to a single page. size_t consumed = 0; - while (consumed < size) { + while (consumed < data.size()) { // Read till the end of the page, at most the remaining number of bytes. - size_t chunk_size = - std::min(size - consumed, page_size - ((ptr + consumed) & page_mask)); + size_t chunk_size = std::min(data.size() - consumed, + page_size - ((ptr + consumed) & page_mask)); remote_iov.push_back({reinterpret_cast(ptr + consumed), chunk_size}); consumed += chunk_size; } - ssize_t result = process_vm_readv(pid, local_iov, ABSL_ARRAYSIZE(local_iov), - remote_iov.data(), remote_iov.size(), 0); + ssize_t result = + is_read ? process_vm_readv(pid, local_iov, ABSL_ARRAYSIZE(local_iov), + remote_iov.data(), remote_iov.size(), 0) + : process_vm_writev(pid, local_iov, ABSL_ARRAYSIZE(local_iov), + remote_iov.data(), remote_iov.size(), 0); if (result < 0) { return absl::ErrnoToStatus( - errno, - absl::StrFormat("process_vm_readv() failed for PID: %d at address: %#x", - pid, ptr)); + errno, absl::StrFormat("transfer() failed for PID: %d at address: %#x", + pid, ptr)); + } + return result; +} + +// Open /proc/pid/mem file descriptor. +absl::StatusOr OpenProcMem(pid_t pid, + bool is_read) { + auto path = absl::StrFormat("/proc/%d/mem", pid); + auto closer = file_util::fileops::FDCloser( + open(path.c_str(), is_read ? O_RDONLY : O_WRONLY)); + if (closer.get() == -1) { + return absl::ErrnoToStatus( + errno, absl::StrFormat("open() failed for PID: %d", pid)); + } + return closer; +} + +absl::StatusOr ProcMemTransfer(bool is_read, pid_t pid, uintptr_t ptr, + absl::Span data) { + if (data.empty()) { + return 0; + } + + SAPI_ASSIGN_OR_RETURN(file_util::fileops::FDCloser fd_closer, + OpenProcMem(pid, is_read)); + size_t total_bytes_transferred = 0; + while (!data.empty()) { + ssize_t bytes_transfered = + is_read ? bytes_transfered = + pread(fd_closer.get(), data.data(), data.size(), ptr) + : bytes_transfered = + pwrite(fd_closer.get(), data.data(), data.size(), ptr); + if (bytes_transfered == 0) { + if (total_bytes_transferred == 0) { + return absl::NotFoundError(absl::StrFormat( + "Transfer was unsuccessful for PID: %d at address: %#x", pid, ptr)); + } + break; + } else if (bytes_transfered < 0) { + if (total_bytes_transferred > 0) { + // Return number of bytes transferred until this error or end. + break; + } + // pread/write of /proc/mem returns EIO when ptr is unmapped. + if (errno == EIO) { + // Emulate returned error code from process_vm_readv. + errno = EFAULT; + } + return absl::ErrnoToStatus( + errno, + absl::StrFormat("transfer() failed for PID: %d at address: %#x", pid, + ptr)); + } + ptr += bytes_transfered; + data = data.subspan(bytes_transfered, data.size() - bytes_transfered); + total_bytes_transferred += bytes_transfered; + } + return total_bytes_transferred; +} + +bool CheckIfProcessVmTransferWorks() { + // Fall-back to pread("/proc/$pid/mem") if process_vm_readv is unavailable. + static bool process_vm_transfer_works = []() { + constexpr char kMagic = 42; + char src = kMagic; + char dst = 0; + absl::StatusOr read = internal::ReadBytesFromPidWithReadv( + getpid(), reinterpret_cast(&src), absl::MakeSpan(&dst, 1)); + if (!read.ok() || *read != 1 || dst != kMagic) { + SAPI_RAW_LOG(WARNING, + "This system does not seem to support the process_vm_readv()" + " or process_vm_writev syscall. Falling back to transfers" + " via /proc/pid/mem."); + return false; + } + return true; + }(); + return process_vm_transfer_works; +} + +} // namespace + +namespace internal { + +absl::StatusOr ReadBytesFromPidWithReadv(pid_t pid, uintptr_t ptr, + absl::Span data) { + return ProcessVmTransfer(true, pid, ptr, data); +} + +absl::StatusOr WriteBytesToPidWithWritev(pid_t pid, uintptr_t ptr, + absl::Span data) { + return ProcessVmTransfer( + false, pid, ptr, + absl::MakeSpan(const_cast(data.data()), data.size())); +} + +absl::StatusOr ReadBytesFromPidWithProcMem(pid_t pid, uintptr_t ptr, + absl::Span data) { + return ProcMemTransfer(true, pid, ptr, data); +} + +absl::StatusOr WriteBytesToPidWithProcMem(pid_t pid, uintptr_t ptr, + absl::Span data) { + return ProcMemTransfer( + false, pid, ptr, + absl::MakeSpan(const_cast(data.data()), data.size())); +} + +} // namespace internal + +absl::StatusOr ReadBytesFromPidInto(pid_t pid, uintptr_t ptr, + absl::Span data) { + if (CheckIfProcessVmTransferWorks()) { + return internal::ReadBytesFromPidWithReadv(pid, ptr, data); + } else { + return internal::ReadBytesFromPidWithProcMem(pid, ptr, data); + } +} + +absl::StatusOr WriteBytesToPidFrom(pid_t pid, uintptr_t ptr, + absl::Span data) { + if (CheckIfProcessVmTransferWorks()) { + return internal::WriteBytesToPidWithWritev(pid, ptr, data); + } else { + return internal::WriteBytesToPidWithProcMem(pid, ptr, data); } +} + +absl::StatusOr> ReadBytesFromPid(pid_t pid, uintptr_t ptr, + size_t size) { + // Allocate enough bytes to hold the entire size. + std::vector bytes(size, 0); + SAPI_ASSIGN_OR_RETURN( + size_t result, + ReadBytesFromPidInto( + pid, ptr, + absl::MakeSpan(reinterpret_cast(bytes.data()), size))); // Ensure only successfully read bytes are returned. bytes.resize(result); return bytes; diff --git a/sandboxed_api/sandbox2/util.h b/sandboxed_api/sandbox2/util.h index 871b2ef1..9e9ac1d1 100644 --- a/sandboxed_api/sandbox2/util.h +++ b/sandboxed_api/sandbox2/util.h @@ -20,6 +20,7 @@ #include +#include #include #include #include @@ -27,6 +28,7 @@ #include "absl/base/attributes.h" #include "absl/base/macros.h" #include "absl/status/statusor.h" +#include "absl/types/span.h" namespace sandbox2::util { @@ -107,9 +109,38 @@ std::string GetRlimitName(int resource); // Returns ptrace event name std::string GetPtraceEventName(int event); +namespace internal { +// Reads `data`'s length of bytes from `ptr` in `pid`, returns number of bytes +// read or an error. +absl::StatusOr ReadBytesFromPidWithReadv(pid_t pid, uintptr_t ptr, + absl::Span data); + +// Writes `data` to `ptr` in `pid`, returns number of bytes written or an error. +absl::StatusOr WriteBytesToPidWithWritev(pid_t pid, uintptr_t ptr, + absl::Span data); + +// Reads `data`'s length of bytes from `ptr` in `pid`, returns number of bytes +// read or an error. +absl::StatusOr ReadBytesFromPidWithProcMem(pid_t pid, uintptr_t ptr, + absl::Span data); + +// Writes `data` to `ptr` in `pid`, returns number of bytes written or an error. +absl::StatusOr WriteBytesToPidWithProcMem(pid_t pid, uintptr_t ptr, + absl::Span data); +}; // namespace internal + +// Reads `data`'s length of bytes from `ptr` in `pid`, returns number of bytes +// read or an error. +absl::StatusOr ReadBytesFromPidInto(pid_t pid, uintptr_t ptr, + absl::Span data); + +// Writes `data` to `ptr` in `pid`, returns number of bytes written or an error. +absl::StatusOr WriteBytesToPidFrom(pid_t pid, uintptr_t remote_ptr, + absl::Span data); + // Reads `size` bytes from the given `ptr` address, or returns an error. absl::StatusOr> ReadBytesFromPid(pid_t pid, uintptr_t ptr, - uint64_t size); + size_t size); // Reads a path string (NUL-terminated, shorter than PATH_MAX) from another // process memory diff --git a/sandboxed_api/sandbox2/util_test.cc b/sandboxed_api/sandbox2/util_test.cc index bf121a43..007d213a 100644 --- a/sandboxed_api/sandbox2/util_test.cc +++ b/sandboxed_api/sandbox2/util_test.cc @@ -28,9 +28,11 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/cleanup/cleanup.h" +#include "absl/log/check.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "absl/types/span.h" #include "sandboxed_api/testing.h" #include "sandboxed_api/util/status_matchers.h" @@ -40,6 +42,7 @@ namespace { using ::sapi::GetTestSourcePath; using ::sapi::IsOk; using ::testing::ElementsAre; +using ::testing::ElementsAreArray; using ::testing::Eq; using ::testing::Gt; using ::testing::IsEmpty; @@ -180,5 +183,214 @@ TEST(CommunitacteTest, Normal) { EXPECT_THAT(output, StartsWith("3\nargv1\nargv2\nenv1\nenv2\n")); } +TEST(ReadBytesFromPidTest, Normal) { + absl::StatusOr> read = ReadBytesFromPid( + getpid(), reinterpret_cast(kTestString.data()), + kTestString.size()); + EXPECT_THAT(*read, ElementsAreArray(kTestString)); +} + +TEST(ReadBytesFromPidTest, NearUnmappedMemory) { + const uintptr_t page_size = getpagesize(); + ASSERT_LE(kTestString.size(), page_size); + char* res = reinterpret_cast(mmap(nullptr, 2 * page_size, + PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0)); + ASSERT_THAT(res, Ne(MAP_FAILED)); + ASSERT_THAT(munmap(&res[page_size], page_size), Eq(0)); + absl::Cleanup cleanup = [res, page_size]() { + ASSERT_THAT(munmap(res, page_size), Eq(0)); + }; + char* data = &res[page_size - kTestString.size()]; + memcpy(data, kTestString.data(), kTestString.size()); + absl::StatusOr> read = ReadBytesFromPid( + getpid(), reinterpret_cast(data), 2 * kTestString.size()); + ASSERT_THAT(read, IsOk()); + EXPECT_THAT(*read, ElementsAreArray(kTestString)); +} + +class MemoryTransferTest : public testing::TestWithParam { + protected: + absl::StatusOr Read(pid_t pid, uintptr_t ptr, absl::Span data) { + if (GetParam()) { + return internal::ReadBytesFromPidWithReadv(pid, ptr, data); + } + return internal::ReadBytesFromPidWithProcMem(pid, ptr, data); + } + absl::StatusOr Write(pid_t pid, uintptr_t ptr, + absl::Span data) { + if (GetParam()) { + return internal::WriteBytesToPidWithWritev(pid, ptr, data); + } + return internal::WriteBytesToPidWithProcMem(pid, ptr, data); + } +}; + +class ReadBytesFromPidIntoTest : public MemoryTransferTest {}; + +TEST_P(ReadBytesFromPidIntoTest, Normal) { + char data[kTestString.size()] = {0}; + absl::StatusOr bytes_read = + Read(getpid(), reinterpret_cast(kTestString.data()), + absl::MakeSpan(data)); + ASSERT_THAT(bytes_read, IsOk()); + EXPECT_THAT(*bytes_read, Eq(kTestString.size())); + EXPECT_THAT(data, ElementsAreArray(kTestString)); +} + +TEST_P(ReadBytesFromPidIntoTest, SplitPage) { + const uintptr_t page_size = getpagesize(); + ASSERT_LE(kTestString.size(), page_size); + char* res = reinterpret_cast(mmap(nullptr, 2 * page_size, + PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0)); + ASSERT_THAT(res, Ne(MAP_FAILED)); + absl::Cleanup cleanup = [res, page_size]() { + ASSERT_THAT(munmap(res, 2 * page_size), Eq(0)); + }; + char* data = &res[page_size - kTestString.size() / 2]; + memcpy(data, kTestString.data(), kTestString.size()); + char output[kTestString.size()]; + absl::StatusOr bytes_read = + Read(getpid(), reinterpret_cast(data), absl::MakeSpan(output)); + ASSERT_THAT(bytes_read, IsOk()); + EXPECT_THAT(*bytes_read, Eq(kTestString.size())); + EXPECT_THAT(output, ElementsAreArray(kTestString)); +} + +TEST_P(ReadBytesFromPidIntoTest, InvalidPid) { + char data; + absl::StatusOr bytes_read = + Read(-1, reinterpret_cast(&data), absl::MakeSpan(&data, 1)); + ASSERT_THAT(bytes_read, Not(IsOk())); +} + +TEST_P(ReadBytesFromPidIntoTest, ZeroLength) { + char data; + absl::StatusOr bytes_read = Read( + getpid(), reinterpret_cast(&data), absl::MakeSpan(&data, 0)); + ASSERT_THAT(bytes_read, IsOk()); + ASSERT_THAT(*bytes_read, Eq(0)); +} + +TEST_P(ReadBytesFromPidIntoTest, ZeroLengthWithInvalidPid) { + char data; + absl::StatusOr bytes_read = + Read(-1, reinterpret_cast(&data), absl::MakeSpan(&data, 0)); + ASSERT_THAT(bytes_read, IsOk()); + ASSERT_THAT(*bytes_read, Eq(0)); +} + +TEST_P(ReadBytesFromPidIntoTest, UnmappedMemory) { + const uintptr_t page_size = getpagesize(); + char* res = + reinterpret_cast(mmap(nullptr, page_size, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0)); + ASSERT_THAT(res, Ne(MAP_FAILED)); + ASSERT_THAT(munmap(res, page_size), Eq(0)); + absl::StatusOr bytes_read = + Read(getpid(), reinterpret_cast(res), absl::MakeSpan(res, 1)); + ASSERT_THAT(bytes_read, Not(IsOk())); +} + +TEST_P(ReadBytesFromPidIntoTest, NearUnmappedMemory) { + const uintptr_t page_size = getpagesize(); + char* res = reinterpret_cast(mmap(nullptr, 2 * page_size, + PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0)); + ASSERT_THAT(res, Ne(MAP_FAILED)); + // Unmap second page so there's a gap. + ASSERT_THAT(munmap(&res[page_size], page_size), Eq(0)); + absl::Cleanup cleanup = [res, page_size]() { + ASSERT_THAT(munmap(res, page_size), Eq(0)); + }; + char* data = &res[page_size - kTestString.size() / 2]; + memcpy(data, kTestString.data(), kTestString.size() / 2); + char output[kTestString.size()]; + absl::StatusOr bytes_read = + Read(getpid(), reinterpret_cast(data), absl::MakeSpan(output)); + ASSERT_THAT(bytes_read, IsOk()); + EXPECT_THAT(*bytes_read, Eq(kTestString.size() / 2)); + EXPECT_THAT(absl::MakeSpan(data, kTestString.size() / 2), + Eq(absl::MakeSpan(kTestString.data(), kTestString.size() / 2))); +} + +INSTANTIATE_TEST_SUITE_P(ReadBytesFromPidInto, ReadBytesFromPidIntoTest, + testing::Values(true, false)); + +class WriteBytesToPidFromTest : public MemoryTransferTest {}; + +TEST_P(WriteBytesToPidFromTest, Normal) { + char data[kTestString.size()] = {0}; + absl::StatusOr bytes_written = + Write(getpid(), reinterpret_cast(data), kTestString); + ASSERT_THAT(bytes_written, IsOk()); + EXPECT_THAT(*bytes_written, Eq(kTestString.size())); + EXPECT_THAT(data, ElementsAreArray(kTestString)); +} + +TEST_P(WriteBytesToPidFromTest, SplitPage) { + const uintptr_t page_size = getpagesize(); + ASSERT_LE(kTestString.size(), page_size); + char* res = reinterpret_cast(mmap(nullptr, 2 * page_size, + PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0)); + ASSERT_THAT(res, Ne(MAP_FAILED)); + absl::Cleanup cleanup = [res, page_size]() { + ASSERT_THAT(munmap(res, 2 * page_size), Eq(0)); + }; + char* data = &res[page_size - kTestString.size() / 2]; + absl::StatusOr bytes_written = + Write(getpid(), reinterpret_cast(data), kTestString); + ASSERT_THAT(bytes_written, IsOk()); + EXPECT_THAT(*bytes_written, Eq(kTestString.size())); + EXPECT_THAT(kTestString, ElementsAreArray(data, kTestString.size())); +} + +TEST_P(WriteBytesToPidFromTest, InvalidPid) { + char data = 0; + absl::StatusOr bytes_written = + Write(-1, reinterpret_cast(&data), absl::MakeSpan(&data, 1)); + ASSERT_THAT(bytes_written, Not(IsOk())); +} + +TEST_P(WriteBytesToPidFromTest, ZeroLength) { + char data = 0; + absl::StatusOr bytes_written = Write( + getpid(), reinterpret_cast(&data), absl::MakeSpan(&data, 0)); + ASSERT_THAT(bytes_written, IsOk()); + ASSERT_THAT(*bytes_written, Eq(0)); +} + +TEST_P(WriteBytesToPidFromTest, ZeroLengthWithInvalidPid) { + char data = 0; + absl::StatusOr bytes_written = + Write(-1, reinterpret_cast(&data), absl::MakeSpan(&data, 0)); + ASSERT_THAT(bytes_written, IsOk()); + ASSERT_THAT(*bytes_written, Eq(0)); +} + +TEST_P(WriteBytesToPidFromTest, NearUnmappedMemory) { + const uintptr_t page_size = getpagesize(); + char* res = reinterpret_cast(mmap(nullptr, 2 * page_size, + PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0)); + ASSERT_THAT(res, Ne(MAP_FAILED)); + ASSERT_THAT(munmap(&res[page_size], page_size), Eq(0)); + absl::Cleanup cleanup = [res, page_size]() { + ASSERT_THAT(munmap(res, page_size), Eq(0)); + }; + char* data = &res[page_size - kTestString.size() / 2]; + absl::StatusOr bytes_written = + Write(getpid(), reinterpret_cast(data), kTestString); + ASSERT_THAT(bytes_written, IsOk()); + EXPECT_THAT(*bytes_written, Eq(kTestString.size() / 2)); + EXPECT_THAT(absl::MakeSpan(data, kTestString.size() / 2), + Eq(absl::MakeSpan(kTestString.data(), kTestString.size() / 2))); +} + +INSTANTIATE_TEST_SUITE_P(WriteBytesToPidFrom, WriteBytesToPidFromTest, + testing::Values(true, false)); + } // namespace } // namespace sandbox2::util diff --git a/sandboxed_api/var_abstract.cc b/sandboxed_api/var_abstract.cc index d4fc4adc..6ddb79b2 100644 --- a/sandboxed_api/var_abstract.cc +++ b/sandboxed_api/var_abstract.cc @@ -19,13 +19,17 @@ #include #include +#include +#include #include #include #include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" +#include "absl/types/span.h" #include "sandboxed_api/rpcchannel.h" +#include "sandboxed_api/sandbox2/util.h" #include "sandboxed_api/util/status_macros.h" #include "sandboxed_api/var_ptr.h" @@ -103,22 +107,12 @@ absl::Status Var::TransferToSandboxee(RPCChannel* rpc_channel, pid_t pid) { absl::StrCat("Object: ", GetTypeString(), " has no remote object set")); } - struct iovec local = { - .iov_base = GetLocal(), - .iov_len = GetSize(), - }; - struct iovec remote = { - .iov_base = GetRemote(), - .iov_len = GetSize(), - }; - - ssize_t ret = process_vm_writev(pid, &local, 1, &remote, 1, 0); - if (ret == -1) { - PLOG(WARNING) << "process_vm_writev(pid: " << pid - << " laddr: " << GetLocal() << " raddr: " << GetRemote() - << " size: " << GetSize() << ")"; - return absl::UnavailableError("process_vm_writev failed"); - } + SAPI_ASSIGN_OR_RETURN( + size_t ret, + sandbox2::util::WriteBytesToPidFrom( + pid, reinterpret_cast(GetRemote()), + absl::MakeSpan(reinterpret_cast(GetLocal()), GetSize()))); + if (ret != GetSize()) { LOG(WARNING) << "process_vm_writev(pid: " << pid << " laddr: " << GetLocal() << " raddr: " << GetRemote() << " size: " << GetSize() << ")" @@ -139,21 +133,11 @@ absl::Status Var::TransferFromSandboxee(RPCChannel* rpc_channel, pid_t pid) { absl::StrCat("Object: ", GetTypeString(), " has no local storage set")); } - struct iovec local = { - .iov_base = GetLocal(), - .iov_len = GetSize(), - }; - struct iovec remote = { - .iov_base = GetRemote(), - .iov_len = GetSize(), - }; - - ssize_t ret = process_vm_readv(pid, &local, 1, &remote, 1, 0); - if (ret == -1) { - PLOG(WARNING) << "process_vm_readv(pid: " << pid << " laddr: " << GetLocal() - << " raddr: " << GetRemote() << " size: " << GetSize() << ")"; - return absl::UnavailableError("process_vm_readv failed"); - } + SAPI_ASSIGN_OR_RETURN( + size_t ret, + sandbox2::util::ReadBytesFromPidInto( + pid, reinterpret_cast(GetRemote()), + absl::MakeSpan(reinterpret_cast(GetLocal()), GetSize()))); if (ret != GetSize()) { LOG(WARNING) << "process_vm_readv(pid: " << pid << " laddr: " << GetLocal() << " raddr: " << GetRemote() << " size: " << GetSize() << ")"