Commit 6cf351e8 authored by Hannes Payer's avatar Hannes Payer Committed by Commit Bot

[heap] Never read out of the [x,y) range during Bitmap operations.

Bug=chromium:852420

Change-Id: Ia810292e4f9592836e7ce734686cadc69328b1c3
Reviewed-on: https://chromium-review.googlesource.com/c/1262475
Commit-Queue: Hannes Payer <hpayer@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56408}
parent b79147d5
...@@ -28,6 +28,9 @@ void Bitmap::MarkAllBits() { ...@@ -28,6 +28,9 @@ void Bitmap::MarkAllBits() {
} }
void Bitmap::SetRange(uint32_t start_index, uint32_t end_index) { void Bitmap::SetRange(uint32_t start_index, uint32_t end_index) {
if (start_index >= end_index) return;
end_index--;
unsigned int start_cell_index = start_index >> Bitmap::kBitsPerCellLog2; unsigned int start_cell_index = start_index >> Bitmap::kBitsPerCellLog2;
MarkBit::CellType start_index_mask = 1u << Bitmap::IndexInCell(start_index); MarkBit::CellType start_index_mask = 1u << Bitmap::IndexInCell(start_index);
unsigned int end_cell_index = end_index >> Bitmap::kBitsPerCellLog2; unsigned int end_cell_index = end_index >> Bitmap::kBitsPerCellLog2;
...@@ -43,10 +46,11 @@ void Bitmap::SetRange(uint32_t start_index, uint32_t end_index) { ...@@ -43,10 +46,11 @@ void Bitmap::SetRange(uint32_t start_index, uint32_t end_index) {
base::Relaxed_Store(cell_base + i, ~0u); base::Relaxed_Store(cell_base + i, ~0u);
} }
// Finally, fill all bits until the end address in the last cell with 1s. // Finally, fill all bits until the end address in the last cell with 1s.
SetBitsInCell<AccessMode::ATOMIC>(end_cell_index, (end_index_mask - 1)); SetBitsInCell<AccessMode::ATOMIC>(end_cell_index,
end_index_mask | (end_index_mask - 1));
} else { } else {
SetBitsInCell<AccessMode::ATOMIC>(start_cell_index, SetBitsInCell<AccessMode::ATOMIC>(
end_index_mask - start_index_mask); start_cell_index, end_index_mask | (end_index_mask - start_index_mask));
} }
// This fence prevents re-ordering of publishing stores with the mark- // This fence prevents re-ordering of publishing stores with the mark-
// bit setting stores. // bit setting stores.
...@@ -54,6 +58,9 @@ void Bitmap::SetRange(uint32_t start_index, uint32_t end_index) { ...@@ -54,6 +58,9 @@ void Bitmap::SetRange(uint32_t start_index, uint32_t end_index) {
} }
void Bitmap::ClearRange(uint32_t start_index, uint32_t end_index) { void Bitmap::ClearRange(uint32_t start_index, uint32_t end_index) {
if (start_index >= end_index) return;
end_index--;
unsigned int start_cell_index = start_index >> Bitmap::kBitsPerCellLog2; unsigned int start_cell_index = start_index >> Bitmap::kBitsPerCellLog2;
MarkBit::CellType start_index_mask = 1u << Bitmap::IndexInCell(start_index); MarkBit::CellType start_index_mask = 1u << Bitmap::IndexInCell(start_index);
...@@ -71,10 +78,11 @@ void Bitmap::ClearRange(uint32_t start_index, uint32_t end_index) { ...@@ -71,10 +78,11 @@ void Bitmap::ClearRange(uint32_t start_index, uint32_t end_index) {
base::Relaxed_Store(cell_base + i, 0); base::Relaxed_Store(cell_base + i, 0);
} }
// Finally, set all bits until the end address in the last cell with 0s. // Finally, set all bits until the end address in the last cell with 0s.
ClearBitsInCell<AccessMode::ATOMIC>(end_cell_index, (end_index_mask - 1)); ClearBitsInCell<AccessMode::ATOMIC>(end_cell_index,
end_index_mask | (end_index_mask - 1));
} else { } else {
ClearBitsInCell<AccessMode::ATOMIC>(start_cell_index, ClearBitsInCell<AccessMode::ATOMIC>(
(end_index_mask - start_index_mask)); start_cell_index, end_index_mask | (end_index_mask - start_index_mask));
} }
// This fence prevents re-ordering of publishing stores with the mark- // This fence prevents re-ordering of publishing stores with the mark-
// bit clearing stores. // bit clearing stores.
...@@ -82,6 +90,9 @@ void Bitmap::ClearRange(uint32_t start_index, uint32_t end_index) { ...@@ -82,6 +90,9 @@ void Bitmap::ClearRange(uint32_t start_index, uint32_t end_index) {
} }
bool Bitmap::AllBitsSetInRange(uint32_t start_index, uint32_t end_index) { bool Bitmap::AllBitsSetInRange(uint32_t start_index, uint32_t end_index) {
if (start_index >= end_index) return false;
end_index--;
unsigned int start_cell_index = start_index >> Bitmap::kBitsPerCellLog2; unsigned int start_cell_index = start_index >> Bitmap::kBitsPerCellLog2;
MarkBit::CellType start_index_mask = 1u << Bitmap::IndexInCell(start_index); MarkBit::CellType start_index_mask = 1u << Bitmap::IndexInCell(start_index);
...@@ -97,21 +108,18 @@ bool Bitmap::AllBitsSetInRange(uint32_t start_index, uint32_t end_index) { ...@@ -97,21 +108,18 @@ bool Bitmap::AllBitsSetInRange(uint32_t start_index, uint32_t end_index) {
for (unsigned int i = start_cell_index + 1; i < end_cell_index; i++) { for (unsigned int i = start_cell_index + 1; i < end_cell_index; i++) {
if (cells()[i] != ~0u) return false; if (cells()[i] != ~0u) return false;
} }
matching_mask = (end_index_mask - 1); matching_mask = end_index_mask | (end_index_mask - 1);
// Check against a mask of 0 to avoid dereferencing the cell after the return ((cells()[end_cell_index] & matching_mask) == matching_mask);
// end of the bitmap.
return (matching_mask == 0) ||
((cells()[end_cell_index] & matching_mask) == matching_mask);
} else { } else {
matching_mask = end_index_mask - start_index_mask; matching_mask = end_index_mask | (end_index_mask - start_index_mask);
// Check against a mask of 0 to avoid dereferencing the cell after the return (cells()[end_cell_index] & matching_mask) == matching_mask;
// end of the bitmap.
return (matching_mask == 0) ||
(cells()[end_cell_index] & matching_mask) == matching_mask;
} }
} }
bool Bitmap::AllBitsClearInRange(uint32_t start_index, uint32_t end_index) { bool Bitmap::AllBitsClearInRange(uint32_t start_index, uint32_t end_index) {
if (start_index >= end_index) return true;
end_index--;
unsigned int start_cell_index = start_index >> Bitmap::kBitsPerCellLog2; unsigned int start_cell_index = start_index >> Bitmap::kBitsPerCellLog2;
MarkBit::CellType start_index_mask = 1u << Bitmap::IndexInCell(start_index); MarkBit::CellType start_index_mask = 1u << Bitmap::IndexInCell(start_index);
...@@ -125,15 +133,11 @@ bool Bitmap::AllBitsClearInRange(uint32_t start_index, uint32_t end_index) { ...@@ -125,15 +133,11 @@ bool Bitmap::AllBitsClearInRange(uint32_t start_index, uint32_t end_index) {
for (unsigned int i = start_cell_index + 1; i < end_cell_index; i++) { for (unsigned int i = start_cell_index + 1; i < end_cell_index; i++) {
if (cells()[i]) return false; if (cells()[i]) return false;
} }
matching_mask = (end_index_mask - 1); matching_mask = end_index_mask | (end_index_mask - 1);
// Check against a mask of 0 to avoid dereferencing the cell after the return !(cells()[end_cell_index] & matching_mask);
// end of the bitmap.
return (matching_mask == 0) || !(cells()[end_cell_index] & matching_mask);
} else { } else {
matching_mask = end_index_mask - start_index_mask; matching_mask = end_index_mask | (end_index_mask - start_index_mask);
// Check against a mask of 0 to avoid dereferencing the cell after the return !(cells()[end_cell_index] & matching_mask);
// end of the bitmap.
return (matching_mask == 0) || !(cells()[end_cell_index] & matching_mask);
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment