Speculative fix for occasional crazy linker crash

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
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288126

[rmcilroy, 2014-08-07 15:41:52]

lgtm

+benm for content OWNER review.

Could you make the title and first line of the description more descriptive
please.

https://codereview.chromium.org/443393003/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
(right):

https://codereview.chromium.org/443393003/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:132:
boolean inServiceProcessWithSharedRelro = false;
nit - is "ServiceProcess" significant here? Maybe just "requestedSharedRelro"?

https://codereview.chromium.org/443393003/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:155:
Log.w(TAG, "Failed to load native library, retrying...");
nit - "Failed to load native library with shared relro, retrying with non-shared
relro".

[rmcilroy, 2014-08-07 15:44:38]

actually +benm for content OWNERS review this time...

[rmcilroy, 2014-08-07 15:44:40]

actually +benm for content OWNERS review this time...

[benm (inactive), 2014-08-07 15:46:55]

lgtm

[simonb (inactive), 2014-08-07 16:42:46]

https://codereview.chromium.org/443393003/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
(right):

https://codereview.chromium.org/443393003/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:132:
boolean inServiceProcessWithSharedRelro = false;
On 2014/08/07 15:41:52, rmcilroy wrote:
> nit - is "ServiceProcess" significant here? Maybe just "requestedSharedRelro"?

Done.

https://codereview.chromium.org/443393003/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:155:
Log.w(TAG, "Failed to load native library, retrying...");
On 2014/08/07 15:41:52, rmcilroy wrote:
> nit - "Failed to load native library with shared relro, retrying with
non-shared
> relro".

Done.

[simonb (inactive), 2014-08-07 17:24:21]

The CQ bit was checked by simonb@chromium.org

[commit-bot: I haz the power, 2014-08-07 17:26:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/simonb@chromium.org/443393003/20001

[commit-bot: I haz the power, 2014-08-07 18:51:10]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[commit-bot: I haz the power, 2014-08-07 19:55:46]

Change committed as 288126

[pasko, 2014-09-10 18:46:17]

pasko@chromium.org changed reviewers:
+ pasko@chromium.org

[pasko, 2014-09-10 18:46:18]

lgtm as a speculative fix, but it's scary for long term.

The original crazylinker design was based on the assumption that
System.loadLibrary() and all other Android goodnesses would never map anything
below 1GB. If we have enough evidence of mappings happening below 1GB (does it
happen on svelte?), we must design it differently.

If we often hit the problem of not finding a place for a 192MiB region, then I
have worries that our attempts to map several regions necessary for library load
will also be in trouble. To load the libchrome DSO we need a few mappings,
roughly for .text, .got+.data, .bss, .rel.ro and some more. How are we doing it
in the crazylinker? These mappings all are forced to be on the constant offset
from the library start address, so if one of the mappings clashes with something
from the system, the library fails to load.

There are ways to mitigate this, but let's figure out what guarantees Android
provides with regards of address space, and then we can implement something more
sustainable, i.e. less of a hack.

If Android does not provide us with sufficient guarantees, we would have to have
a fallback to System.loadLibrary(), which means: no loading from the .apk, no
packed relocations, etc.

https://codereview.chromium.org/443393003/diff/20001/third_party/android_craz...
File third_party/android_crazy_linker/src/src/crazy_linker_elf_loader.cpp
(right):

https://codereview.chromium.org/443393003/diff/20001/third_party/android_craz...
third_party/android_crazy_linker/src/src/crazy_linker_elf_loader.cpp:208: return
false;
Random suggestion:

Printing /proc/self/maps to logcat when we return false from here would help
debugging a lot!

[simonb (inactive), 2014-09-11 14:21:21]

See also:

https://codereview.chromium.org/470053003/