Fix PartialCircularBuffer OOB memcpy().

chrome/common/partial_circular_buffer.cc

Index: chrome/common/partial_circular_buffer.cc
diff --git a/chrome/common/partial_circular_buffer.cc b/chrome/common/partial_circular_buffer.cc
index 4161cc1864706b2ddbce9037d26c72ecf0203e48..3f9795f8a02650d3c1d6627718fddb313a86a8a0 100644
--- a/chrome/common/partial_circular_buffer.cc
+++ b/chrome/common/partial_circular_buffer.cc
@@ -139,30 +139,45 @@ uint32 PartialCircularBuffer::Read(void* buffer, uint32 buffer_size) {
 
 void PartialCircularBuffer::Write(const void* buffer, uint32 buffer_size) {
   DCHECK(buffer_data_);
-  uint32 position_before_write = position_;
+  const uint8* input = static_cast<const uint8*>(buffer);
+  uint32 wrap_position = buffer_data_->wrap_position;
+  uint32 cycle_size = data_size_ - wrap_position;
+
+  // First write the non-wrapping part.
+  if (position_ < wrap_position) {
+    uint32 space_left = wrap_position - position_;
+    uint32 write_size = std::min(buffer_size, space_left);
+    DoWrite(input, write_size);
+    input += write_size;
+    buffer_size -= write_size;
+  }
 
-  uint32 to_eof = data_size_ - position_;
-  uint32 to_write = std::min(buffer_size, to_eof);
-  DoWrite(buffer_data_->data + position_, buffer, to_write);
-  if (position_ >= data_size_) {
-    DCHECK_EQ(position_, data_size_);
-    position_ = buffer_data_->wrap_position;
+  // Skip the part that would overlap.
+  if (buffer_size > cycle_size) {
+    uint32 skip = buffer_size - cycle_size;
+    input += skip;
+    buffer_size -= skip;
+    position_ = wrap_position + (position_ - wrap_position + skip) % cycle_size;
   }
 
-  if (to_write < buffer_size) {
-    uint32 remainder_to_write = buffer_size - to_write;
-    DCHECK_LT(position_, position_before_write);
-    DCHECK_LE(position_ + remainder_to_write, position_before_write);
-    DoWrite(buffer_data_->data + position_,
-            reinterpret_cast<const uint8*>(buffer) + to_write,
-            remainder_to_write);
+  // Finally write the wrapping part.

[Nico, 2015/04/08 23:13:05]

Say that this loop will run at most twice.

[gzobqq, 2015/04/09 08:36:40]

Done.

+  while (buffer_size > 0) {
+    uint32 written = DoWrite(input, buffer_size);
+    input += written;
+    buffer_size -= written;
   }
 }
 
-void PartialCircularBuffer::DoWrite(void* dest, const void* src, uint32 num) {
-  memcpy(dest, src, num);
-  position_ += num;
+uint32 PartialCircularBuffer::DoWrite(const uint8* input, uint32 input_size) {
+  DCHECK_LT(position_, data_size_);
+  uint32 space_left = data_size_ - position_;
+  uint32 write_size = std::min(input_size, space_left);
+  memcpy(buffer_data_->data + position_, input, write_size);
+  position_ += write_size;
   buffer_data_->total_written =
-      std::min(buffer_data_->total_written + num, data_size_);
+      std::min(buffer_data_->total_written + write_size, data_size_);
+  if (position_ >= data_size_)

[Nico, 2015/04/08 23:13:05]

If this happens, it will always be ==, not >=, right? Maybe keep the
DCHECK_EQ(position_, data_size_) in this if?

[gzobqq, 2015/04/09 08:36:40]

Done.

+    position_ = buffer_data_->wrap_position;
   buffer_data_->end_position = position_;
+  return write_size;
 }