Resize IPC input buffer to fit the next message.

Sometimes we can get IPC message size from its header. In those cases
we resize IPC::ChannelReader' overflow buffer to fit the entire message
to avoid growing / reallocating it as we receive message's data.

BUG=529940

Review URL: https://codereview.chromium.org/1345353004

Cr-Commit-Position: refs/heads/master@{#351586}

base/pickle.cc

@@ -7,6 +7,7 @@
 #include <stdlib.h>
 
 #include <algorithm>  // for max()
+#include <limits>
 
 #include "base/bits.h"
 #include "base/macros.h"
@@ -338,17 +339,41 @@ size_t Pickle::GetTotalAllocatedSize() const {
 const char* Pickle::FindNext(size_t header_size,
                              const char* start,
                              const char* end) {
+  size_t pickle_size = 0;
+  if (!PeekNext(header_size, start, end, &pickle_size))
+    return NULL;
+
+  if (pickle_size > static_cast<size_t>(end - start))
+    return NULL;
+
+  return start + pickle_size;
+}
+
+// static
+bool Pickle::PeekNext(size_t header_size,
+                      const char* start,
+                      const char* end,
+                      size_t* pickle_size) {
   DCHECK_EQ(header_size, bits::Align(header_size, sizeof(uint32)));
+  DCHECK_GE(header_size, sizeof(Header));
   DCHECK_LE(header_size, static_cast<size_t>(kPayloadUnit));
 
   size_t length = static_cast<size_t>(end - start);
   if (length < sizeof(Header))
-    return NULL;
+    return false;
 
   const Header* hdr = reinterpret_cast<const Header*>(start);
-  if (length < header_size || length - header_size < hdr->payload_size)
-    return NULL;
-  return start + header_size + hdr->payload_size;
+  if (length < header_size)
+    return false;
+
+  if (hdr->payload_size > std::numeric_limits<size_t>::max() - header_size) {
+    // If payload_size causes an overflow, we return maximum possible
+    // pickle size to indicate that.
+    *pickle_size = std::numeric_limits<size_t>::max();
+  } else {
+    *pickle_size = header_size + hdr->payload_size;
+  }
+  return true;
 }
 
 template <size_t length> void Pickle::WriteBytesStatic(const void* data) {

base/pickle.h

@@ -271,6 +271,18 @@ class BASE_EXPORT Pickle {
                               const char* range_start,
                               const char* range_end);
 
+  // Parse pickle header and return total size of the pickle. Data range
+  // doesn't need to contain entire pickle.
+  // Returns true if pickle header was found and parsed. Callers must check
+  // returned |pickle_size| for sanity (against maximum message size, etc).
+  // NOTE: when function successfully parses a header, but encounters an
+  // overflow during pickle size calculation, it sets |pickle_size| to the
+  // maximum size_t value and returns true.
+  static bool PeekNext(size_t header_size,
+                       const char* range_start,
+                       const char* range_end,
+                       size_t* pickle_size);
+
   // The allocation granularity of the payload.
   static const int kPayloadUnit;
 
@@ -298,6 +310,8 @@ class BASE_EXPORT Pickle {
 
   FRIEND_TEST_ALL_PREFIXES(PickleTest, DeepCopyResize);
   FRIEND_TEST_ALL_PREFIXES(PickleTest, Resize);
+  FRIEND_TEST_ALL_PREFIXES(PickleTest, PeekNext);
+  FRIEND_TEST_ALL_PREFIXES(PickleTest, PeekNextOverflow);
   FRIEND_TEST_ALL_PREFIXES(PickleTest, FindNext);
   FRIEND_TEST_ALL_PREFIXES(PickleTest, FindNextWithIncompleteHeader);
   FRIEND_TEST_ALL_PREFIXES(PickleTest, FindNextOverflow);

base/pickle_unittest.cc

@@ -233,6 +233,88 @@ TEST(PickleTest, BadLenStr16) {
   EXPECT_FALSE(iter.ReadString16(&outstr));
 }
 
+TEST(PickleTest, PeekNext) {
+  struct CustomHeader : base::Pickle::Header {
+    int cookies[10];
+  };
+
+  Pickle pickle(sizeof(CustomHeader));
+
+  EXPECT_TRUE(pickle.WriteString("Goooooooooooogle"));
+
+  const char* pickle_data = static_cast<const char*>(pickle.data());
+
+  size_t pickle_size;
+
+  // Data range doesn't contain header
+  EXPECT_FALSE(Pickle::PeekNext(
+      sizeof(CustomHeader),
+      pickle_data,
+      pickle_data + sizeof(CustomHeader) - 1,
+      &pickle_size));
+
+  // Data range contains header
+  EXPECT_TRUE(Pickle::PeekNext(
+      sizeof(CustomHeader),
+      pickle_data,
+      pickle_data + sizeof(CustomHeader),
+      &pickle_size));
+  EXPECT_EQ(pickle_size, pickle.size());
+
+  // Data range contains header and some other data
+  EXPECT_TRUE(Pickle::PeekNext(
+      sizeof(CustomHeader),
+      pickle_data,
+      pickle_data + sizeof(CustomHeader) + 1,
+      &pickle_size));
+  EXPECT_EQ(pickle_size, pickle.size());
+
+  // Data range contains full pickle
+  EXPECT_TRUE(Pickle::PeekNext(
+      sizeof(CustomHeader),
+      pickle_data,
+      pickle_data + pickle.size(),
+      &pickle_size));
+  EXPECT_EQ(pickle_size, pickle.size());
+}
+
+TEST(PickleTest, PeekNextOverflow) {
+  struct CustomHeader : base::Pickle::Header {
+    int cookies[10];
+  };
+
+  CustomHeader header;
+
+  // Check if we can wrap around at all
+  if (sizeof(size_t) > sizeof(header.payload_size))
+    return;
+
+  const char* pickle_data = reinterpret_cast<const char*>(&header);
+
+  size_t pickle_size;
+
+  // Wrapping around is detected and reported as maximum size_t value
+  header.payload_size = static_cast<uint32_t>(
+      1 - static_cast<int32_t>(sizeof(CustomHeader)));
+  EXPECT_TRUE(Pickle::PeekNext(
+      sizeof(CustomHeader),
+      pickle_data,
+      pickle_data + sizeof(CustomHeader),
+      &pickle_size));
+  EXPECT_EQ(pickle_size, std::numeric_limits<size_t>::max());
+
+  // Ridiculous pickle sizes are fine (callers are supposed to
+  // verify them)
+  header.payload_size =
+      std::numeric_limits<uint32_t>::max() / 2 - sizeof(CustomHeader);
+  EXPECT_TRUE(Pickle::PeekNext(
+      sizeof(CustomHeader),
+      pickle_data,
+      pickle_data + sizeof(CustomHeader),
+      &pickle_size));
+  EXPECT_EQ(pickle_size, std::numeric_limits<uint32_t>::max() / 2);
+}
+
 TEST(PickleTest, FindNext) {
   Pickle pickle;
   EXPECT_TRUE(pickle.WriteInt(1));

ipc/ipc_channel_reader.cc

@@ -78,17 +78,15 @@ bool ChannelReader::TranslateInputData(const char* input_data,
     p = input_data;
     end = input_data + input_data_len;
   } else {
-    if (input_overflow_buf_.size() + input_data_len >
-        Channel::kMaximumMessageSize) {
-      input_overflow_buf_.clear();
-      LOG(ERROR) << "IPC message is too big";
+    if (!CheckMessageSize(input_overflow_buf_.size() + input_data_len))
       return false;
-    }
     input_overflow_buf_.append(input_data, input_data_len);
     p = input_overflow_buf_.data();
     end = p + input_overflow_buf_.size();
   }
 
+  size_t next_message_size = 0;
+
   // Dispatch all complete messages in the data buffer.
   while (p < end) {
     Message::NextMessageInfo info;
@@ -127,13 +125,27 @@ bool ChannelReader::TranslateInputData(const char* input_data,
       p = info.message_end;
     } else {
       // Last message is partial.
+      next_message_size = info.message_size;
+      if (!CheckMessageSize(next_message_size))
+        return false;
       break;
     }
   }
 
   // Save any partial data in the overflow buffer.
   input_overflow_buf_.assign(p, end - p);
 
+  if (!input_overflow_buf_.empty()) {
+    // We have something in the overflow buffer, which means that we will
+    // append the next data chunk (instead of parsing it directly). So we
+    // resize the buffer to fit the next message, to avoid repeatedly
+    // growing the buffer as we receive all message' data chunks.
+    next_message_size += Channel::kReadBufferSize - 1;
+    if (next_message_size > input_overflow_buf_.capacity()) {
+      input_overflow_buf_.reserve(next_message_size);
+    }
+  }
+
   if (input_overflow_buf_.empty() && !DidEmptyInputBuffers())
     return false;
   return true;
@@ -249,5 +261,14 @@ void ChannelReader::StopObservingAttachmentBroker() {
 #endif  // USE_ATTACHMENT_BROKER
 }
 
+bool ChannelReader::CheckMessageSize(size_t size) {
+  if (size <= Channel::kMaximumMessageSize) {
+    return true;
+  }
+  input_overflow_buf_.clear();
+  LOG(ERROR) << "IPC message is too big: " << size;
+  return false;
+}
+
 }  // namespace internal
 }  // namespace IPC

ipc/ipc_channel_reader.h

@@ -127,6 +127,8 @@ class IPC_EXPORT ChannelReader : public SupportsAttachmentBrokering,
  private:
   FRIEND_TEST_ALL_PREFIXES(ChannelReaderTest, AttachmentAlreadyBrokered);
   FRIEND_TEST_ALL_PREFIXES(ChannelReaderTest, AttachmentNotYetBrokered);
+  FRIEND_TEST_ALL_PREFIXES(ChannelReaderTest, ResizeOverflowBuffer);
+  FRIEND_TEST_ALL_PREFIXES(ChannelReaderTest, InvalidMessageSize);
 
   typedef std::set<BrokerableAttachment::AttachmentId> AttachmentIdSet;
 
@@ -156,6 +158,9 @@ class IPC_EXPORT ChannelReader : public SupportsAttachmentBrokering,
   void StartObservingAttachmentBroker();
   void StopObservingAttachmentBroker();
 
+  // Checks that |size| is a valid message size. Has side effects if it's not.
+  bool CheckMessageSize(size_t size);
+
   Listener* listener_;
 
   // We read from the pipe into this buffer. Managed by DispatchInputData, do

ipc/ipc_channel_reader_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "build/build_config.h"
 
+#include <limits>
 #include <set>
 
 #include "ipc/attachment_broker.h"
@@ -12,12 +13,13 @@
 #include "ipc/placeholder_brokerable_attachment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if USE_ATTACHMENT_BROKER
 namespace IPC {
 namespace internal {
 
 namespace {
 
+#if USE_ATTACHMENT_BROKER
+
 class MockAttachment : public BrokerableAttachment {
  public:
   MockAttachment() {}
@@ -53,6 +55,8 @@ class MockAttachmentBroker : public AttachmentBroker {
   }
 };
 
+#endif  // USE_ATTACHMENT_BROKER
+
 class MockChannelReader : public ChannelReader {
  public:
   MockChannelReader()
@@ -92,8 +96,16 @@ class MockChannelReader : public ChannelReader {
   AttachmentBroker* broker_;
 };
 
+class ExposedMessage: public Message {
+ public:
+  using Message::Header;
+  using Message::header;
+};
+
 }  // namespace
 
+#if USE_ATTACHMENT_BROKER
+
 TEST(ChannelReaderTest, AttachmentAlreadyBrokered) {
   MockAttachmentBroker broker;
   MockChannelReader reader;
@@ -129,6 +141,55 @@ TEST(ChannelReaderTest, AttachmentNotYetBrokered) {
   EXPECT_EQ(m, reader.get_last_dispatched_message());
 }
 
+#endif  // USE_ATTACHMENT_BROKER
+
+#if !USE_ATTACHMENT_BROKER
+
+// We can determine message size from its header (and hence resize the buffer)
+// only when attachment broker is not used, see IPC::Message::FindNext().
+
+TEST(ChannelReaderTest, ResizeOverflowBuffer) {
+  MockChannelReader reader;
+
+  ExposedMessage::Header header = {};
+
+  header.payload_size = 128 * 1024;
+  EXPECT_LT(reader.input_overflow_buf_.capacity(), header.payload_size);
+  EXPECT_TRUE(reader.TranslateInputData(
+      reinterpret_cast<const char*>(&header), sizeof(header)));
+
+  // Once message header is available we resize overflow buffer to
+  // fit the entire message.
+  EXPECT_GE(reader.input_overflow_buf_.capacity(), header.payload_size);
+}
+
+TEST(ChannelReaderTest, InvalidMessageSize) {
+  MockChannelReader reader;
+
+  ExposedMessage::Header header = {};
+
+  size_t capacity_before = reader.input_overflow_buf_.capacity();
+
+  // Message is slightly larger than maximum allowed size
+  header.payload_size = Channel::kMaximumMessageSize + 1;
+  EXPECT_FALSE(reader.TranslateInputData(
+      reinterpret_cast<const char*>(&header), sizeof(header)));
+  EXPECT_LE(reader.input_overflow_buf_.capacity(), capacity_before);
+
+  // Payload size is negative, overflow is detected by Pickle::PeekNext()
+  header.payload_size = static_cast<uint32_t>(-1);
+  EXPECT_FALSE(reader.TranslateInputData(
+      reinterpret_cast<const char*>(&header), sizeof(header)));
+  EXPECT_LE(reader.input_overflow_buf_.capacity(), capacity_before);
+
+  // Payload size is maximum int32 value
+  header.payload_size = std::numeric_limits<int32_t>::max();
+  EXPECT_FALSE(reader.TranslateInputData(
+      reinterpret_cast<const char*>(&header), sizeof(header)));
+  EXPECT_LE(reader.input_overflow_buf_.capacity(), capacity_before);
+}
+
+#endif  // !USE_ATTACHMENT_BROKER
+
 }  // namespace internal
 }  // namespace IPC
-#endif  // USE_ATTACHMENT_BROKER