Add defensive code in pickle to preclude realloc of shared header_ memory....

base/pickle.cc

 // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/pickle.h"
 
 #include <stdlib.h>
 
 #include <limits>
 #include <string>
@@ skipping 46 matching lines @@
   bool resized = Resize(payload_size);
   CHECK(resized);  // Realloc failed.
   memcpy(header_, other.header_, payload_size);
 }
 
 Pickle::~Pickle() {
   if (capacity_ != kCapacityReadOnly)
     free(header_);
 }
 
 Pickle& Pickle::operator=(const Pickle& other) {

[cpu_(ooo_6.6-7.5), 2009/08/03 16:18:57]

this operator is lacking a check against self assignment.

[jar (doing other things), 2009/08/03 20:11:22]

I'll create a CL that handles this explicitly.

-  if (header_size_ != other.header_size_ && capacity_ != kCapacityReadOnly) {
+  if (capacity_ == kCapacityReadOnly) {
+    header_ = NULL;
+    capacity_ = 0;
+  }
+  if (header_size_ != other.header_size_) {
     free(header_);
     header_ = NULL;
     header_size_ = other.header_size_;
   }
-  bool resized = Resize(other.header_size_ + other.header_->payload_size);
+  bool resized = Resize(header_size_ + other.header_->payload_size);

[cpu_(ooo_6.6-7.5), 2009/08/03 16:18:57]

I don't get the change in line 73. It seems more clear the previous version.
What about changing line 79 so the two match?

[jar (doing other things), 2009/08/03 20:11:22]

The goal was to have the argument of the memcpy (two lines from now) have
EXACTLY the same size argument as we have in the resize (which was not the case
before). 

It can be deduced from the last 4 lines that they have to be the same, but it
seemed better to use the same text in both users (resize and memcpy).

If you'd rather I use the longer one in both cases, I can do that.  I'll add it
to the CL containing more defensive code for the self-assignment.

   CHECK(resized);  // Realloc failed.
   memcpy(header_, other.header_, header_size_ + other.header_->payload_size);
   variable_buffer_offset_ = other.variable_buffer_offset_;
   return *this;
 }
 
 bool Pickle::ReadBool(void** iter, bool* result) const {
   DCHECK(iter);
 
   int tmp;
@@ skipping 274 matching lines @@
   }
 
   // Update the payload size and variable buffer size
   header_->payload_size -= (*cur_length - new_length);
   *cur_length = new_length;
 }
 
 bool Pickle::Resize(size_t new_capacity) {
   new_capacity = AlignInt(new_capacity, kPayloadUnit);
 
+  CHECK(capacity_ != kCapacityReadOnly);
   void* p = realloc(header_, new_capacity);
   if (!p)
     return false;
 
   header_ = reinterpret_cast<Header*>(p);
   capacity_ = new_capacity;
   return true;
 }
 
 // static
 const char* Pickle::FindNext(size_t header_size,
                              const char* start,
                              const char* end) {
   DCHECK(header_size == AlignInt(header_size, sizeof(uint32)));
   DCHECK(header_size <= static_cast<size_t>(kPayloadUnit));
 
   const Header* hdr = reinterpret_cast<const Header*>(start);
   const char* payload_base = start + header_size;
   const char* payload_end = payload_base + hdr->payload_size;
   if (payload_end < payload_base)
     return NULL;
 
   return (payload_end > end) ? NULL : payload_end;
 }