Bug 1490972 - Limit the number of bytes poisoned to avoid quadratic behavior. r=froydnj

XPCOM strings mark logically unused parts of nsStringBuffer as uninitialized
in debug builds by writing a marker byte and if memory checking is active,
by telling the memory checking that the range of memory is uninitialized.

This patch limits such marking to up to 16 code units to avoid quadratic
behavior, which is especially bad when there's a large disparity between
length and capacity (after a call to SetCapacity()).

The assumption here is that even a small poisoned memory range is enough
to detect the bugs that the poisoning is intended to detect.

MozReview-Commit-ID: 178rp0ckztj

Differential Revision: https://phabricator.services.mozilla.com/D5838

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Henri Sivonen 2018-09-14 18:58:15 +00:00
parent 89ca07c012
commit 13e736d609
2 changed files with 32 additions and 15 deletions

View File

@ -128,14 +128,19 @@ nsTSubstring<T>::StartBulkWriteImpl(size_type aCapacity,
this->mData + aOldSuffixStart,
aSuffixLength);
if (aSuffixLength) {
char_traits::uninitialize(this->mData + aPrefixToPreserve,
aNewSuffixStart - aPrefixToPreserve);
char_traits::uninitialize(this->mData + aNewSuffixStart + aSuffixLength,
curCapacity + 1 - aNewSuffixStart -
aSuffixLength);
char_traits::uninitialize(
this->mData + aPrefixToPreserve,
XPCOM_MIN(size_t(aNewSuffixStart - aPrefixToPreserve),
kNsStringBufferMaxPoison));
char_traits::uninitialize(
this->mData + aNewSuffixStart + aSuffixLength,
XPCOM_MIN(size_t(curCapacity + 1 - aNewSuffixStart - aSuffixLength),
kNsStringBufferMaxPoison));
} else {
char_traits::uninitialize(this->mData + aPrefixToPreserve,
curCapacity + 1 - aPrefixToPreserve);
char_traits::uninitialize(
this->mData + aPrefixToPreserve,
XPCOM_MIN(size_t(curCapacity + 1 - aPrefixToPreserve),
kNsStringBufferMaxPoison));
}
return curCapacity;
}
@ -235,14 +240,19 @@ nsTSubstring<T>::StartBulkWriteImpl(size_type aCapacity,
char_traits::move(
newData + aNewSuffixStart, oldData + aOldSuffixStart, aSuffixLength);
if (aSuffixLength) {
char_traits::uninitialize(this->mData + aPrefixToPreserve,
aNewSuffixStart - aPrefixToPreserve);
char_traits::uninitialize(this->mData + aNewSuffixStart + aSuffixLength,
newCapacity + 1 - aNewSuffixStart -
aSuffixLength);
char_traits::uninitialize(
this->mData + aPrefixToPreserve,
XPCOM_MIN(size_t(aNewSuffixStart - aPrefixToPreserve),
kNsStringBufferMaxPoison));
char_traits::uninitialize(
this->mData + aNewSuffixStart + aSuffixLength,
XPCOM_MIN(size_t(newCapacity + 1 - aNewSuffixStart - aSuffixLength),
kNsStringBufferMaxPoison));
} else {
char_traits::uninitialize(this->mData + aPrefixToPreserve,
newCapacity + 1 - aPrefixToPreserve);
char_traits::uninitialize(
this->mData + aPrefixToPreserve,
XPCOM_MIN(size_t(newCapacity + 1 - aPrefixToPreserve),
kNsStringBufferMaxPoison));
}
} else {
char_traits::copy(newData, oldData, aPrefixToPreserve);

View File

@ -24,6 +24,11 @@
#error "Using XPCOM strings is limited to code linked into libxul."
#endif
// The max number of logically uninitialized code units to
// fill with a marker byte or to mark as unintialized for
// memory checking. (Limited to avoid quadratic behavior.)
const size_t kNsStringBufferMaxPoison = 16;
template <typename T> class nsTSubstringSplitter;
template <typename T> class nsTString;
template <typename T> class nsTSubstring;
@ -1393,7 +1398,9 @@ private:
// counting after the zero terminator the we just wrote above,
// we end up overwriting the space for terminator not reflected
// in the capacity number.
char_traits::uninitialize(base_string_type::mData + aLength + 1, Capacity() - aLength);
char_traits::uninitialize(
base_string_type::mData + aLength + 1,
XPCOM_MIN(size_t(Capacity() - aLength), kNsStringBufferMaxPoison));
#endif
}