0

Speculative fix for occasional crazy linker crash.

crbug/397634 describes crashes that occur occasionally in the crazy linker
when loading the browser shared library.  These crashes have been observed
only in the M37 beta.

The current theory is that part of the address range that the crazy linker
must map with MAP_FIXED for shared RELRO to work may, on occasions,
already be mapped.  If it is mapped then the crazy linker's MAP_FIXED
to reserve space will overwrite it, with unpleasant results.  The crazy
linker tries to use a range of addresses that is 'safe' from use by the
system, but it may not be watertight.

This fix should work round the problem by removing the MAP_FIXED from
the mmap that reserves space, so that if any part of the range is already
occupied then the mmap returns a valid mapping, just not at the requested
address.  If that happens the fixed address load fails, and the
ChildProcessService retries with a non-fixed address.

This is mildly suboptimal since it loses the gain from shared RELRO, but
the library will still load and run.  Occurrences of this in the M37
beta suggest that it will be a relatively rare event.

So far the bug reported by the beta has not shown up in testing, but
something a little similar can be simulated by deliberately moving the
crazy linker's wanted address to around the heap.  With this perturbation
in place and without this fix, the browser crashes.  When this fix is
added the browser/renderer library loads and runs, and correctly reports
back-out and retry in logcat.

BUG=397634

Review URL: https://codereview.chromium.org/443393003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288126 0039d316-1c4b-4281-b951-d872f2087c98
This commit is contained in:
simonb@chromium.org
2014-08-07 19:55:36 +00:00
parent 6a14e28bd1
commit 22aa4f5f85
3 changed files with 34 additions and 7 deletions
content/public/android/java/src/org/chromium/content/app
third_party/android_crazy_linker

@ -129,7 +129,7 @@ public class ChildProcessService extends Service {
public void run() {
try {
boolean useLinker = Linker.isUsed();
boolean requestedSharedRelro = false;
if (useLinker) {
synchronized (mMainThread) {
while (!mIsBound) {
@ -137,18 +137,37 @@ public class ChildProcessService extends Service {
}
}
if (mLinkerParams != null) {
if (mLinkerParams.mWaitForSharedRelro)
if (mLinkerParams.mWaitForSharedRelro) {
requestedSharedRelro = true;
Linker.initServiceProcess(mLinkerParams.mBaseLoadAddress);
else
} else {
Linker.disableSharedRelros();
}
Linker.setTestRunnerClassName(mLinkerParams.mTestRunnerClassName);
}
}
boolean isLoaded = false;
try {
LibraryLoader.loadNow(getApplicationContext(), false);
isLoaded = true;
} catch (ProcessInitException e) {
Log.e(TAG, "Failed to load native library, exiting child process", e);
if (requestedSharedRelro) {
Log.w(TAG, "Failed to load native library with shared RELRO, " +
"retrying without");
} else {
Log.e(TAG, "Failed to load native library", e);
}
}
if (!isLoaded && requestedSharedRelro) {
Linker.disableSharedRelros();
try {
LibraryLoader.loadNow(getApplicationContext(), false);
isLoaded = true;
} catch (ProcessInitException e) {
Log.e(TAG, "Failed to load native library on retry", e);
}
}
if (!isLoaded) {
System.exit(-1);
}
synchronized (mMainThread) {

@ -35,3 +35,5 @@ Local Modifications:
- Move packed relocation dynamic tags from DT_LOPROC range to DT_LOOS range.
- Add support for x86_64.
- Speculative fix for crbug/397634.

@ -181,7 +181,9 @@ bool ElfLoader::ReadProgramHeader(Error* error) {
// segments of a program header table. This is done by creating a
// private anonymous mmap() with PROT_NONE.
//
// This will use the wanted_load_address_ value,
// This will use the wanted_load_address_ value. Fails if the requested
// address range cannot be reserved. Typically this would be because
// it overlaps an existing, possibly system, mapping.
bool ElfLoader::ReserveAddressSpace(Error* error) {
ELF::Addr min_vaddr;
load_size_ =
@ -197,7 +199,6 @@ bool ElfLoader::ReserveAddressSpace(Error* error) {
// Support loading at a fixed address.
if (wanted_load_address_) {
addr = static_cast<uint8_t*>(wanted_load_address_);
mmap_flags |= MAP_FIXED;
}
LOG("%s: address=%p size=%p\n", __FUNCTION__, addr, load_size_);
@ -206,6 +207,11 @@ bool ElfLoader::ReserveAddressSpace(Error* error) {
error->Format("Could not reserve %d bytes of address space", load_size_);
return false;
}
if (wanted_load_address_ && start != addr) {
error->Format("Could not map at %p requested, backing out", addr);
munmap(start, load_size_);
return false;
}
load_start_ = start;
load_bias_ = reinterpret_cast<ELF::Addr>(start) - min_vaddr;