diff --git a/sandboxed_api/sandbox2/comms.cc b/sandboxed_api/sandbox2/comms.cc index f1557a2..00d140a 100644 --- a/sandboxed_api/sandbox2/comms.cc +++ b/sandboxed_api/sandbox2/comms.cc @@ -242,19 +242,26 @@ bool Comms::SendTLV(uint32_t tag, size_t length, const void* value) { SAPI_RAW_VLOG(3, "Sending a TLV message, tag: 0x%08x, length: %zu", tag, length); - { - absl::MutexLock lock(&tlv_send_transmission_mutex_); - if (!Send(&tag, sizeof(tag))) { - return false; - } - if (!Send(&length, sizeof(length))) { - return false; - } - if (length > 0 && !Send(value, length)) { + + // To maintain consistency with `RecvTL()`, we wrap `tag` and `length` in a TL + // struct. + const InternalTLV tl = { + .tag = tag, + .len = length, + }; + + absl::MutexLock lock(&tlv_send_transmission_mutex_); + if (length + sizeof(tl) > kSendTLVTempBufferSize) { + if (!Send(&tl, sizeof(tl))) { return false; } + return Send(value, length); } - return true; + uint8_t tlv[kSendTLVTempBufferSize]; + memcpy(tlv, &tl, sizeof(tl)); + memcpy(reinterpret_cast(tlv) + sizeof(tl), value, length); + + return Send(&tlv, sizeof(tl) + length); } bool Comms::RecvString(std::string* v) { @@ -573,14 +580,13 @@ bool Comms::Recv(void* data, size_t len) { // Internal helper method (low level). bool Comms::RecvTL(uint32_t* tag, size_t* length) { - if (!Recv(reinterpret_cast(tag), sizeof(*tag))) { - SAPI_RAW_VLOG(2, "RecvTL: Can't read tag"); - return false; - } - if (!Recv(reinterpret_cast(length), sizeof(*length))) { - SAPI_RAW_VLOG(2, "RecvTL: Can't read length for tag %u", *tag); + InternalTLV tl; + if (!Recv(reinterpret_cast(&tl), sizeof(tl))) { + SAPI_RAW_VLOG(2, "RecvTL: Can't read tag and length"); return false; } + *tag = tl.tag; + *length = tl.len; if (*length > GetMaxMsgSize()) { SAPI_RAW_LOG(ERROR, "Maximum TLV message size exceeded: (%zu > %zd)", *length, GetMaxMsgSize()); @@ -622,6 +628,7 @@ bool Comms::RecvTLVGeneric(uint32_t* tag, T* value) { bool Comms::RecvTLV(uint32_t* tag, size_t* length, void* buffer, size_t buffer_size) { absl::MutexLock lock(&tlv_recv_transmission_mutex_); + if (!RecvTL(tag, length)) { return false; } diff --git a/sandboxed_api/sandbox2/comms.h b/sandboxed_api/sandbox2/comms.h index 184c1c0..af6ce6d 100644 --- a/sandboxed_api/sandbox2/comms.h +++ b/sandboxed_api/sandbox2/comms.h @@ -84,6 +84,12 @@ class Comms { // sandbox2::Comms object at the server-side). static constexpr int kSandbox2ClientCommsFD = 1023; + // Within SendTLV, a stack-allocated buffer is created to contiguously store + // the TLV in order to perform one call to Send. + // If the TLV is larger than the size below, two calls to Send are + // used. + static constexpr size_t kSendTLVTempBufferSize = 1024; + static constexpr DefaultConnectionTag kDefaultConnection = {}; static constexpr const char* kSandbox2CommsFDEnvVar = "SANDBOX2_COMMS_FD"; @@ -229,12 +235,14 @@ class Comms { // State of the channel (enum), socket will have to be connected later on. State state_ = State::kUnconnected; - // Special struct for passing credentials or FDs. Different from the one above - // as it inlines the value. This is important as the data is transmitted using - // sendmsg/recvmsg instead of send/recv. + // Special struct for passing credentials or FDs. + // When passing credentials or FDs, it inlines the value. This is important as + // the data is transmitted using sendmsg/recvmsg instead of send/recv. + // It is also used when sending/receiving through SendTLV/RecvTLV to reduce + // writes/reads, although the value is written/read separately. struct ABSL_ATTRIBUTE_PACKED InternalTLV { uint32_t tag; - uint32_t len; + size_t len; }; // Fills sockaddr_un struct with proper values.