Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(805)

Unified Diff: chrome/common/partial_circular_buffer.cc

Issue 1061053002: Fix PartialCircularBuffer OOB memcpy(). (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressed comments Created 5 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 >=, rig
gzobqq 2015/04/09 08:36:40 Done.
+ position_ = buffer_data_->wrap_position;
buffer_data_->end_position = position_;
+ return write_size;
}

Powered by Google App Engine
This is Rietveld 408576698