Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-23 04:41:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1506910 - Initialize the poison page with a static initializer. r=glandium,decoder

Browse Source 
Poison was setup at the start of xpcom init when that was assumed to be early enough.
Since then, Poison was added to Maybe, and Maybe has been used everywhere, including in
our channel implementation. As a result, poison was being used before it was initialized.

This basically meant our poison pointers were being replaced with null instead, which
dances into some more UB than accessing a page we have actually allocated. Also, tsan
noticed that accesses to the value were racing with the initializer actually being
called!

A (dynamic) static initializer forces the poison initialization as we can reasonably
hope without getting CallOnce or singleton patterns involved.

Other changes:
  * Cleaned up the outdated documentation for mozWritePoison (the alignment
    restriction was removed in Bug 1414901)
  * Removed the poison supression from TSan

Differential Revision: https://phabricator.services.mozilla.com/D94251
This commit is contained in:
Alexis Beingessner 2020-10-28 20:38:42 +00:00
parent 7e7a9d78e0
commit f045afd928
4 changed files with 20 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
mfbt/Poison.cpp
View File

@ -26,12 +26,6 @@
# endif
#endif
extern "C" {
uintptr_t gMozillaPoisonValue;
uintptr_t gMozillaPoisonBase;
uintptr_t gMozillaPoisonSize;
}
// Freed memory is filled with a poison value, which we arrange to
// form a pointer either to an always-unmapped region of the address
// space, or to a page that has been reserved and rendered
@ -169,12 +163,23 @@ static uintptr_t ReservePoisonArea(uintptr_t rgnsize) {
MOZ_CRASH("no usable poison region identified");
}
void mozPoisonValueInit() {
gMozillaPoisonSize = GetDesiredRegionSize();
gMozillaPoisonBase = ReservePoisonArea(gMozillaPoisonSize);
if (gMozillaPoisonSize == 0) { // can't happen
return;
static uintptr_t GetPoisonValue(uintptr_t aBase, uintptr_t aSize) {
if (aSize == 0) { // can't happen
return 0;
}
gMozillaPoisonValue = gMozillaPoisonBase + gMozillaPoisonSize / 2 - 1;
return aBase + aSize / 2 - 1;
}
// Poison is used so pervasively throughout the codebase that we decided it was
// best to actually use ordered dynamic initialization of globals (AKA static
// constructors) for this. This way everything will have properly initialized
// poison -- except other dynamic initialization code in libmozglue, which there
// shouldn't be much of. (libmozglue is one of the first things loaded, and
// specifically comes before libxul, so nearly all gecko code runs strictly
// after this.)
extern "C" {
uintptr_t gMozillaPoisonSize = GetDesiredRegionSize();
uintptr_t gMozillaPoisonBase = ReservePoisonArea(gMozillaPoisonSize);
uintptr_t gMozillaPoisonValue =
GetPoisonValue(gMozillaPoisonBase, gMozillaPoisonSize);
}

11
mfbt/Poison.h
View File

@ -29,9 +29,8 @@ inline uintptr_t mozPoisonValue() { return gMozillaPoisonValue; }
/**
* Overwrite the memory block of aSize bytes at aPtr with the poison value.
* aPtr MUST be aligned at a sizeof(uintptr_t) boundary.
* Only an even number of sizeof(uintptr_t) bytes are overwritten, the last
* few bytes (if any) is not overwritten.
* Only a multiple of sizeof(uintptr_t) bytes are overwritten, the last
* few bytes (if any) are not overwritten.
*/
inline void mozWritePoison(void* aPtr, size_t aSize) {
const uintptr_t POISON = mozPoisonValue();
@ -43,12 +42,6 @@ inline void mozWritePoison(void* aPtr, size_t aSize) {
}
}
/**
* Initialize the poison value.
* This should only be called once.
*/
extern MFBT_API void mozPoisonValueInit();
/* Values annotated by CrashReporter */
extern MFBT_DATA uintptr_t gMozillaPoisonBase;
extern MFBT_DATA uintptr_t gMozillaPoisonSize;

3
mozglue/build/TsanOptions.cpp
View File

@ -221,9 +221,6 @@ extern "C" const char* __tsan_default_suppressions() {
// Bug 1506812
"race:BeginBackgroundRead\n"
// Bug 1506910
"race:gMozillaPoisonValue\n"
// Bug 1601286
"race:setFlagBit\n"
"race:isFatInline\n"

3
xpcom/build/XPCOMInit.cpp
View File

@ -259,8 +259,6 @@ NS_InitXPCOM(nsIServiceManager** aResult, nsIFile* aBinDirectory,
sInitialized = true;
mozPoisonValueInit();
NS_LogInit();
NS_InitAtomTable();
@ -495,7 +493,6 @@ NS_InitXPCOM(nsIServiceManager** aResult, nsIFile* aBinDirectory,
EXPORT_XPCOM_API(nsresult)
NS_InitMinimalXPCOM() {
mozPoisonValueInit();
NS_SetMainThread();
mozilla::TimeStamp::Startup();
NS_LogInit();