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

Side by Side 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 unified diff | Download patch
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/common/partial_circular_buffer.h" 5 #include "chrome/common/partial_circular_buffer.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 8
9 #include "base/logging.h" 9 #include "base/logging.h"
10 10
(...skipping 43 matching lines...) Expand 10 before | Expand all | Expand 10 after
54 DCHECK_LT(buffer_data_->wrap_position, data_size_); 54 DCHECK_LT(buffer_data_->wrap_position, data_size_);
55 position_ = buffer_data_->end_position; 55 position_ = buffer_data_->end_position;
56 } else { 56 } else {
57 DCHECK_LT(wrap_position, data_size_); 57 DCHECK_LT(wrap_position, data_size_);
58 buffer_data_->total_written = 0; 58 buffer_data_->total_written = 0;
59 buffer_data_->wrap_position = wrap_position; 59 buffer_data_->wrap_position = wrap_position;
60 buffer_data_->end_position = 0; 60 buffer_data_->end_position = 0;
61 } 61 }
62 } 62 }
63 63
64 uint32 PartialCircularBuffer::Read(void* buffer, uint32 buffer_size) { 64 uint32 PartialCircularBuffer::Read(void* buffer, uint32 buffer_size) {
Nico 2015/04/08 23:13:05 Ah, hmm, this doesn't allow reading more than the
65 DCHECK(buffer_data_); 65 DCHECK(buffer_data_);
66 if (total_read_ >= buffer_data_->total_written) 66 if (total_read_ >= buffer_data_->total_written)
67 return 0; 67 return 0;
68 68
69 uint8* buffer_uint8 = reinterpret_cast<uint8*>(buffer); 69 uint8* buffer_uint8 = reinterpret_cast<uint8*>(buffer);
70 uint32 read = 0; 70 uint32 read = 0;
71 71
72 // Read from beginning part. 72 // Read from beginning part.
73 if (position_ < buffer_data_->wrap_position) { 73 if (position_ < buffer_data_->wrap_position) {
74 uint32 to_wrap_pos = buffer_data_->wrap_position - position_; 74 uint32 to_wrap_pos = buffer_data_->wrap_position - position_;
(...skipping 57 matching lines...) Expand 10 before | Expand all | Expand 10 after
132 position_ += to_read; 132 position_ += to_read;
133 total_read_ += to_read; 133 total_read_ += to_read;
134 read += to_read; 134 read += to_read;
135 DCHECK_LE(read, buffer_size); 135 DCHECK_LE(read, buffer_size);
136 DCHECK_LE(total_read_, buffer_data_->total_written); 136 DCHECK_LE(total_read_, buffer_data_->total_written);
137 return read; 137 return read;
138 } 138 }
139 139
140 void PartialCircularBuffer::Write(const void* buffer, uint32 buffer_size) { 140 void PartialCircularBuffer::Write(const void* buffer, uint32 buffer_size) {
141 DCHECK(buffer_data_); 141 DCHECK(buffer_data_);
142 uint32 position_before_write = position_; 142 const uint8* input = static_cast<const uint8*>(buffer);
143 uint32 wrap_position = buffer_data_->wrap_position;
144 uint32 cycle_size = data_size_ - wrap_position;
143 145
144 uint32 to_eof = data_size_ - position_; 146 // First write the non-wrapping part.
145 uint32 to_write = std::min(buffer_size, to_eof); 147 if (position_ < wrap_position) {
146 DoWrite(buffer_data_->data + position_, buffer, to_write); 148 uint32 space_left = wrap_position - position_;
147 if (position_ >= data_size_) { 149 uint32 write_size = std::min(buffer_size, space_left);
148 DCHECK_EQ(position_, data_size_); 150 DoWrite(input, write_size);
149 position_ = buffer_data_->wrap_position; 151 input += write_size;
152 buffer_size -= write_size;
150 } 153 }
151 154
152 if (to_write < buffer_size) { 155 // Skip the part that would overlap.
153 uint32 remainder_to_write = buffer_size - to_write; 156 if (buffer_size > cycle_size) {
154 DCHECK_LT(position_, position_before_write); 157 uint32 skip = buffer_size - cycle_size;
155 DCHECK_LE(position_ + remainder_to_write, position_before_write); 158 input += skip;
156 DoWrite(buffer_data_->data + position_, 159 buffer_size -= skip;
157 reinterpret_cast<const uint8*>(buffer) + to_write, 160 position_ = wrap_position + (position_ - wrap_position + skip) % cycle_size;
158 remainder_to_write); 161 }
162
163 // 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.
164 while (buffer_size > 0) {
165 uint32 written = DoWrite(input, buffer_size);
166 input += written;
167 buffer_size -= written;
159 } 168 }
160 } 169 }
161 170
162 void PartialCircularBuffer::DoWrite(void* dest, const void* src, uint32 num) { 171 uint32 PartialCircularBuffer::DoWrite(const uint8* input, uint32 input_size) {
163 memcpy(dest, src, num); 172 DCHECK_LT(position_, data_size_);
164 position_ += num; 173 uint32 space_left = data_size_ - position_;
174 uint32 write_size = std::min(input_size, space_left);
175 memcpy(buffer_data_->data + position_, input, write_size);
176 position_ += write_size;
165 buffer_data_->total_written = 177 buffer_data_->total_written =
166 std::min(buffer_data_->total_written + num, data_size_); 178 std::min(buffer_data_->total_written + write_size, data_size_);
179 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.
180 position_ = buffer_data_->wrap_position;
167 buffer_data_->end_position = position_; 181 buffer_data_->end_position = position_;
182 return write_size;
168 } 183 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698