From 08a956a415d6c90d1349c416693c2f79ab521a0c Mon Sep 17 00:00:00 2001 From: Wiktor Garbacz Date: Fri, 18 Sep 2020 06:48:30 -0700 Subject: [PATCH] Use opaque void* instead of uint8_t* in Comms PiperOrigin-RevId: 332441641 Change-Id: I09902e98726a0bd57b47d3454ddcb6ef05021d56 --- sandboxed_api/rpcchannel.cc | 33 +++++++++++---------------------- sandboxed_api/sandbox2/comms.cc | 17 +++++++++-------- sandboxed_api/sandbox2/comms.h | 8 ++++---- 3 files changed, 24 insertions(+), 34 deletions(-) diff --git a/sandboxed_api/rpcchannel.cc b/sandboxed_api/rpcchannel.cc index 66e1b2b..8b19547 100644 --- a/sandboxed_api/rpcchannel.cc +++ b/sandboxed_api/rpcchannel.cc @@ -27,8 +27,7 @@ namespace sapi { absl::Status RPCChannel::Call(const FuncCall& call, uint32_t tag, FuncRet* ret, v::Type exp_type) { absl::MutexLock lock(&mutex_); - if (!comms_->SendTLV(tag, sizeof(call), - reinterpret_cast(&call))) { + if (!comms_->SendTLV(tag, sizeof(call), &call)) { return absl::UnavailableError("Sending TLV value failed"); } SAPI_ASSIGN_OR_RETURN(auto fret, Return(exp_type)); @@ -67,9 +66,7 @@ absl::StatusOr RPCChannel::Return(v::Type exp_type) { absl::Status RPCChannel::Allocate(size_t size, void** addr) { absl::MutexLock lock(&mutex_); - uint64_t sz = size; - if (!comms_->SendTLV(comms::kMsgAllocate, sizeof(sz), - reinterpret_cast(&sz))) { + if (!comms_->SendTLV(comms::kMsgAllocate, sizeof(size), &size)) { return absl::UnavailableError("Sending TLV value failed"); } @@ -86,7 +83,7 @@ absl::Status RPCChannel::Reallocate(void* old_addr, size_t size, req.size = size; if (!comms_->SendTLV(comms::kMsgReallocate, sizeof(comms::ReallocRequest), - reinterpret_cast(&req))) { + &req)) { return absl::UnavailableError("Sending TLV value failed"); } @@ -104,8 +101,7 @@ absl::Status RPCChannel::Reallocate(void* old_addr, size_t size, absl::Status RPCChannel::Free(void* addr) { absl::MutexLock lock(&mutex_); uint64_t remote = reinterpret_cast(addr); - if (!comms_->SendTLV(comms::kMsgFree, sizeof(remote), - reinterpret_cast(&remote))) { + if (!comms_->SendTLV(comms::kMsgFree, sizeof(remote), &remote)) { return absl::UnavailableError("Sending TLV value failed"); } @@ -118,8 +114,7 @@ absl::Status RPCChannel::Free(void* addr) { absl::Status RPCChannel::Symbol(const char* symname, void** addr) { absl::MutexLock lock(&mutex_); - if (!comms_->SendTLV(comms::kMsgSymbol, strlen(symname) + 1, - reinterpret_cast(symname))) { + if (!comms_->SendTLV(comms::kMsgSymbol, strlen(symname) + 1, symname)) { return absl::UnavailableError("Sending TLV value failed"); } @@ -137,9 +132,8 @@ absl::Status RPCChannel::Exit() { // Try the RPC exit sequence. But, the only thing that matters as a success // indicator is whether the Comms channel had been closed - bool unused = true; - comms_->SendTLV(comms::kMsgExit, sizeof(unused), - reinterpret_cast(&unused)); + comms_->SendTLV(comms::kMsgExit, 0, nullptr); + bool unused; comms_->RecvBool(&unused); if (!comms_->IsTerminated()) { @@ -154,9 +148,7 @@ absl::Status RPCChannel::Exit() { absl::Status RPCChannel::SendFD(int local_fd, int* remote_fd) { absl::MutexLock lock(&mutex_); - bool unused = true; - if (!comms_->SendTLV(comms::kMsgSendFd, sizeof(unused), - reinterpret_cast(&unused))) { + if (!comms_->SendTLV(comms::kMsgSendFd, 0, nullptr)) { return absl::UnavailableError("Sending TLV value failed"); } if (!comms_->SendFD(local_fd)) { @@ -173,8 +165,7 @@ absl::Status RPCChannel::SendFD(int local_fd, int* remote_fd) { absl::Status RPCChannel::RecvFD(int remote_fd, int* local_fd) { absl::MutexLock lock(&mutex_); - if (!comms_->SendTLV(comms::kMsgRecvFd, sizeof(remote_fd), - reinterpret_cast(&remote_fd))) { + if (!comms_->SendTLV(comms::kMsgRecvFd, sizeof(remote_fd), &remote_fd)) { return absl::UnavailableError("Sending TLV value failed"); } @@ -191,8 +182,7 @@ absl::Status RPCChannel::RecvFD(int remote_fd, int* local_fd) { absl::Status RPCChannel::Close(int remote_fd) { absl::MutexLock lock(&mutex_); - if (!comms_->SendTLV(comms::kMsgClose, sizeof(remote_fd), - reinterpret_cast(&remote_fd))) { + if (!comms_->SendTLV(comms::kMsgClose, sizeof(remote_fd), &remote_fd)) { return absl::UnavailableError("Sending TLV value failed"); } @@ -205,8 +195,7 @@ absl::Status RPCChannel::Close(int remote_fd) { absl::StatusOr RPCChannel::Strlen(void* str) { absl::MutexLock lock(&mutex_); - if (!comms_->SendTLV(comms::kMsgStrlen, sizeof(str), - reinterpret_cast(&str))) { + if (!comms_->SendTLV(comms::kMsgStrlen, sizeof(str), &str)) { return absl::UnavailableError("Sending TLV value failed"); } diff --git a/sandboxed_api/sandbox2/comms.cc b/sandboxed_api/sandbox2/comms.cc index 5195a9d..9b2f719 100644 --- a/sandboxed_api/sandbox2/comms.cc +++ b/sandboxed_api/sandbox2/comms.cc @@ -228,7 +228,7 @@ void Comms::Terminate() { } } -bool Comms::SendTLV(uint32_t tag, uint64_t length, const uint8_t* bytes) { +bool Comms::SendTLV(uint32_t tag, uint64_t length, const void* value) { if (length > GetMaxMsgSize()) { SAPI_RAW_LOG(ERROR, "Maximum TLV message size exceeded: (%u > %u)", length, GetMaxMsgSize()); @@ -249,14 +249,14 @@ bool Comms::SendTLV(uint32_t tag, uint64_t length, const uint8_t* bytes) { length); { absl::MutexLock lock(&tlv_send_transmission_mutex_); - if (!Send(reinterpret_cast(&tag), sizeof(tag))) { + if (!Send(&tag, sizeof(tag))) { return false; } - if (!Send(reinterpret_cast(&length), sizeof(length))) { + if (!Send(&length, sizeof(length))) { return false; } if (length > 0) { - if (!Send(bytes, length)) { + if (!Send(value, length)) { return false; } } @@ -279,8 +279,7 @@ bool Comms::RecvString(std::string* v) { } bool Comms::SendString(const std::string& v) { - return SendTLV(kTagString, v.length(), - reinterpret_cast(v.c_str())); + return SendTLV(kTagString, v.length(), v.c_str()); } bool Comms::RecvBytes(std::vector* buffer) { @@ -513,8 +512,9 @@ socklen_t Comms::CreateSockaddrUn(sockaddr_un* sun) { return slen; } -bool Comms::Send(const uint8_t* bytes, uint64_t len) { +bool Comms::Send(const void* data, uint64_t len) { uint64_t total_sent = 0; + const char* bytes = reinterpret_cast(data); const auto op = [bytes, len, &total_sent](int fd) -> ssize_t { PotentiallyBlockingRegion region; return TEMP_FAILURE_RETRY(write(fd, &bytes[total_sent], len - total_sent)); @@ -545,8 +545,9 @@ bool Comms::Send(const uint8_t* bytes, uint64_t len) { return true; } -bool Comms::Recv(uint8_t* bytes, uint64_t len) { +bool Comms::Recv(void* data, uint64_t len) { uint64_t total_recv = 0; + char* bytes = reinterpret_cast(data); const auto op = [bytes, len, &total_recv](int fd) -> ssize_t { PotentiallyBlockingRegion region; return TEMP_FAILURE_RETRY(read(fd, &bytes[total_recv], len - total_recv)); diff --git a/sandboxed_api/sandbox2/comms.h b/sandboxed_api/sandbox2/comms.h index 7863288..530943e 100644 --- a/sandboxed_api/sandbox2/comms.h +++ b/sandboxed_api/sandbox2/comms.h @@ -105,7 +105,7 @@ class Comms { // avoid protobuf serialization issues. uint64_t GetMaxMsgSize() const { return std::numeric_limits::max(); } - bool SendTLV(uint32_t tag, uint64_t length, const uint8_t* bytes); + bool SendTLV(uint32_t tag, uint64_t length, const void* value); // Receive a TLV structure, the memory for the value will be allocated // by std::vector. bool RecvTLV(uint32_t* tag, std::vector* value); @@ -190,8 +190,8 @@ class Comms { socklen_t CreateSockaddrUn(sockaddr_un* sun); // Support for EINTR and size completion. - bool Send(const uint8_t* bytes, uint64_t len); - bool Recv(uint8_t* bytes, uint64_t len); + bool Send(const void* data, uint64_t len); + bool Recv(void* data, uint64_t len); // Receives tag and length. Assumes that the `tlv_transmission_mutex_` mutex // is locked. @@ -212,7 +212,7 @@ class Comms { template bool SendGeneric(T value, uint32_t tag) { - return SendTLV(tag, sizeof(T), reinterpret_cast(&value)); + return SendTLV(tag, sizeof(T), &value); } };