Android: Add chrome-specific dynamic linker.

content/public/android/java/src/org/chromium/content/common/Linker.java

Index: content/public/android/java/src/org/chromium/content/common/Linker.java
diff --git a/content/public/android/java/src/org/chromium/content/common/Linker.java b/content/public/android/java/src/org/chromium/content/common/Linker.java
new file mode 100644
index 0000000000000000000000000000000000000000..d9bdb8fa23731bbb5b435d93c315c9fe3125e306
--- /dev/null
+++ b/content/public/android/java/src/org/chromium/content/common/Linker.java
@@ -0,0 +1,746 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.content.common;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Random;
+import java.util.Set;
+
+import android.content.Context;
+import android.os.Bundle;
+import android.os.Parcel;
+import android.os.Parcelable;
+import android.os.ParcelFileDescriptor;
+import android.util.Log;
+
+// TODO(digit): Move this to org.chromium.content.common
+import org.chromium.content.app.NativeLibraries;
+
+/*
+ * Technical note:
+ *
+ * The point of this class is to provide an alternative to System.loadLibrary()
+ * to load native shared libraries. One specific feature that it supports is the
+ * ability to save RAM by sharing the ELF RELRO sections between renderer
+ * processes.
+ *
+ * When two processes load the same native library at the _same_ memory address,
+ * the content of their RELRO section (which includes C++ vtables or any
+ * constants that contain pointers) will be largely identical [1].
+ *
+ * By default, the RELRO section is backed by private RAM in each process,
+ * which is still significant on mobile (e.g. 1.28 MB / process on Chrome 30 for
+ * Android).
+ *
+ * However, it is possible to save RAM by creating a shared memory region,
+ * copy the RELRO content into it, then have each process swap its private,
+ * regular RELRO, with a shared, read-only, mapping of the shared one.
+ *
+ * This trick saves 98% of the RELRO section size per extra process, after the
+ * first one. On the other hand, this requires careful communication between
+ * the process where the shared RELRO is created and the one(s) where it is used.
+ *
+ * Note that swapping the regular RELRO with the shared one is not an atomic
+ * operation. Care must be taken that no other thread tries to run native code
+ * that accesses it during it. In practice, this means the swap must happen

[palmer, 2013/09/18 19:48:21]

How do you guarantee/check for this? We (jln and I) could really use it for
applying Seccomp-BPF policies. As far as we can tell, the Android Zygote spawns
various threads in the child before our code gets to have a say. Those threads
might do anything...

[digit1, 2013/09/19 17:23:50]

This is simply guaranteed by the library loader that uses this API. See
LibraryLoader changes.

Note that there are still have other Java threads running when this happen, the
guarantee is that none of these can actually run the native code from the loaded
libraries before the RELRO creation.

I believe this does not apply to your situation, which requires that no other
threads in the process exist before setting up the Seccomp-BPF policies, which
is probably impossible to enforce unless you modify the Zygote, and probably the
framework's initialization path :-(

+ * before library native code is executed.
+ *
+ * Finally, note that for security reasons, the browser process shall _not_
+ * load its native libraries at the same addresses than the service ones.
+ *
+ * [1] The exceptions are pointers to external, randomized, symbols, like
+ * those from some system libraries, but these are very few in practice.
+ */
+
+/*
+ * Security considerations:
+ *
+ * - The browser process _never_ load its native libraries at the same
+ *   addresses as the service ones (and there is no sharing / RAM saving
+ *   between the browser and other processes). This makes exploiting
+ *   browser-side buffer-overflows more difficult from a compromised

[palmer, 2013/09/18 19:48:21]

Not just buffer overflows; also use-after-free and so on. I would say simply
"This maintains the efficacy of ASLR as an exploit mitigation across the render
<-> browser privilege boundary."

[digit1, 2013/09/19 17:23:50]

Good point. Done.

+ *   service process.
+ *
+ * - The shared RELRO memory region is always forced read-only after creation,
+ *   which means it is impossible for a compromised service process to map
+ *   it read-write (e.g. by calling mmap() or mprotect()) and modify its
+ *   content, altering values seen in other service processes.
+ *
+ * - Unfortunately, certain Android systems use an old, buggy kernel, that
+ *   doesn't check Ashmem region permissions correctly. See CVE-2011-1149
+ *   for details. This linker probes the system on startup and will completely
+ *   disable shared RELROs if it detects the problem. For the record, this is

[palmer, 2013/09/18 19:48:21]

Thank you. :)

Question: Are the low-end devices we want to run on still running kernels that
old?

[digit1, 2013/09/19 17:23:50]

No, not that old, the issue  was discovered by running the unit tests for the
crazy_linker library inside emulators (to check the x86 and MIPS
implementations), and these have a really dated kernel. For real devices, the
issue seems to have been fixed after Gingerbread, and Chromium doesn't try to
target anything before 4.0.1.

And for the record, we do a lot of performance work using humble Nexus S devices
running 4.1 or 4.2, that's as low end as we can get.

Still, I think it's important to keep the runtime check, just in case :)

+ *   common for Android emulator system images (which are still based on 2.6.29)
+ *
+ * - Once the RELRO ashmem region is mapped into a service process' address
+ *   space, the corresponding file descriptor is immediately closed. The
+ *   file descriptor is kept opened in the browser process, because a copy needs
+ *   to be sent to each new potential service process.
+ *
+ * - The common library load addresses are randomized for each instance of
+ *   the program on the device. See computeRandomBaseLoadAddress() for more
+ *   details on how this is computed.
+ *
+ * - When loading several libraries in service processes, a simple incremental
+ *   approach from the original random base load address is used. This is
+ *   sufficient to deal correctly with component builds (which can use dozens
+ *   of shared libraries), while regular builds always embed a single shared
+ *   library per APK.
+ */
+
+/*
+ * Here's an explanation of how this class is supposed to be used:
+ *
+ *  - Native shared libraries should be loaded with Linker.loadLibrary(),
+ *    instead of System.loadLibrary(). The two functions take the same parameter
+ *    and should behave the same (at a high level).
+ *
+ *  - Before loading any library, prepareLibraryLoad() should be called.

[palmer, 2013/09/18 19:48:21]

Is there a way to enforce/check that this happens correctly?

[digit1, 2013/09/19 17:23:50]

Currently, this is done by the LibraryLoader class, but I've added an assert.

+ *
+ *  - After loading all libraries, finishLibraryLoad() should be called, before
+ *    running any native code from any of the libraries (except their static
+ *    constructors, which can't be avoided).
+ *
+ *  - A service process shall call either wantSharedRelros() or
+ *    disableSharedRelros() early (i.e. before any loadLibrary() call).
+ *    Otherwise, the linker considers that it is running inside the browser
+ *    process. This is because various content-based projects have vastly
+ *    different initialization paths.
+ *
+ *    disableSharedRelros() completely disables shared RELROs, and loadLibrary()
+ *    will behave exactly like System.loadLibrary().
+ *
+ *    wantSharedRelros(baseLoadAddress) indicates that shared RELROs are to be
+ *    used in this process.
+ *
+ *  - The browser is in charge of deciding where in memory each library should
+ *    be loaded in the service processes. This address must be passed to each
+ *    service process (see LinkerParams.java for a helper class to do so).
+ *
+ *  - The browser will also generate shared RELROs for each library it loads.
+ *    More specifically, when in the browser process, the linker will:
+ *
+ *       - Load libraries randomly (just like System.loadLibrary())
+ *       - Compute the fixed address to be used to load the same library
+ *         in service processes.
+ *       - Create a shared memory region populated with the RELRO region
+ *         content pre-relocated for the specific fixed address above.
+ *
+ *    Note that these shared RELRO regions cannot be used inside the browser
+ *    process. They are also never mapped into it.
+ *
+ *  - Once all libraries are loaded in the browser process, one can call
+ *    getSharedRelros() which returns a Bundle instance containing a map that
+ *    links each loaded library to its shared RELRO region.
+ *
+ *    This Bundle must be passed to each service process, for example through
+ *    a Binder call (note that the Bundle includes file descriptors and cannot
+ *    be added as an Intent extra).
+ *
+ *  - In a service process, finishLibraryLoad() will block until the RELRO
+ *    section Bundle is received. This is typically done by calling
+ *    useSharedRelros() from another thread.
+ *
+ *    This method also ensures the process uses the shared RELROs.
+ */
+public class Linker {
+
+    // Log tag for this class. This must match the name of the linker's native library.
+    private static final String TAG = "chrome_android_linker";
+
+    // Set to true to enable debug logs.
+    private static final boolean DEBUG = false;
+
+    // Becomes true after linker initialization.
+    private static boolean sInitialized = false;
+
+    // Set to true to indicate that the system supports safe sharing of RELRO sections.
+    private static boolean sRelroSharingSupported = false;
+
+    // Determines whether this process shall create shared RELRO regions.
+    private static boolean sCreateSharedRelros = true;
+
+    // Becomes true to indicate this process wants to use shared RELRO sections from another one.
+    private static boolean sWantSharedRelros = false;
+
+    // The map of all RELRO sections either created or used in this process.
+    private static Bundle sSharedRelros = null;
+
+    // Current common random base load address.
+    private static long sBaseLoadAddress = 0;
+
+    // Current fixed-location load address for the next library called by loadLibrary().
+    private static long sCurrentLoadAddress = 0;
+
+    // Becomes true if any library fails to load at a given, non-0, fixed address.
+    private static boolean sLoadAtFixedAddressFailed = false;
+
+    // Used internally to initialize the linker's static data. Assume lock is held.

[palmer, 2013/09/18 19:48:21]

Can we enforce rather than assume?

[digit1, 2013/09/19 17:23:50]

I've added an assert Thread.holdsLock() call here, but generally, methods named
with a Locked() suffix indicate that they work on protected data assuming the
lock is held. This is a huge convention for Chromium on Android (and the Android
platform itself), and is used all over the place :-)

+    private static void ensureInitializedLocked() {
+        if (!sInitialized) {
+            if (!sInitialized) {

[palmer, 2013/09/18 19:48:21]

What is the purpose of this?

[digit1, 2013/09/19 17:23:50]

Nothing really, bad refactoring let this through :-) Also found by findbugs
apparently. Fixed.

+                sRelroSharingSupported = false;
+                if (NativeLibraries.USE_CHROME_LINKER) {
+                    if (DEBUG) Log.i(TAG, "Loading lib" + TAG + ".so");
+                    try {
+                        System.loadLibrary(TAG);
+                    } catch (UnsatisfiedLinkError  e) {
+                        // In a component build, the ".cr" suffix is added to each library name.
+                        System.loadLibrary(TAG + ".cr");
+                    }
+                    sRelroSharingSupported = nativeCanUseSharedRelro();
+                    if (!sRelroSharingSupported)
+                        Log.w(TAG, "This system cannot safely share RELRO sections");
+                    else {
+                        if (DEBUG) Log.i(TAG, "This system supports safe shared RELRO sections");
+                    }
+                }
+
+                if (!sRelroSharingSupported) {
+                    // Sanity.
+                    sCreateSharedRelros = false;
+                    sWantSharedRelros = false;
+                }
+
+                sInitialized = true;
+            }
+        }
+    }
+
+    /**
+     * Call this method to determine if this content-based project must
+     * use this linker. If not, System.loadLibrary() should be used to load
+     * libraries instead.
+     */
+    public static boolean isUsed() {
+        // Only GYP targets that are APKs and have the 'use_chrome_linker' variable
+        // defined as 1 will use this linker. For all others (the default), the
+        // auto-generate NativeLibraries.USE_CHROME_LINKER variable will be false.
+        if (!NativeLibraries.USE_CHROME_LINKER)
+            return false;
+
+        synchronized (Linker.class) {
+            ensureInitializedLocked();
+            // At the moment, there is also no point in using this linker if the
+            // system does not support RELRO sharing safely.
+            return sRelroSharingSupported;
+        }
+    }
+
+    /**
+     * Call this method just before loading any native shared libraries in this process.
+     */
+    public static void prepareLibraryLoad() {
+        if (DEBUG) Log.i(TAG, "prepareLibraryLoad() called");

[palmer, 2013/09/18 19:48:21]

Use Log.d instead of checking |DEBUG|?

[digit1, 2013/09/19 17:23:50]

That's a really bad idea. The documentations states that Log.d() are "compiled
in but stripped at runtime" but this is misleading. What really happens is that
the Log.d() implementation does nothing on production devices (it works when you
run the program on a userdebug image, i.e. when running in the emulator, or
doing Android system development).

However, the function call, and any operation needed to craft its parameter is
still in the application code, and you don't want that when DEBUG is set to
false.

Log.v() is similarly unuseful here, so Log.i() is needed, controlled explicitely
by the "if (DEBUG) ...". More on this below.

+        synchronized (Linker.class) {
+            if (sCreateSharedRelros) {
+                // Force generation of random base load address, as well
+                // as creation of shared RELRO sections in this process.
+                setupBaseLoadAddressLocked();
+            }
+        }
+    }
+
+    /**
+     * Call this method just after loading all native shared libraries in this process.
+     * Note that when in a service process, this will block until the RELRO bundle is
+     * received, i.e. when another thread calls useSharedRelros().
+     */
+     public static void finishLibraryLoad() {
+        if (DEBUG) Log.i(TAG, "finishLibraryLoad() called");

[palmer, 2013/09/18 19:48:21]

Log.d here too?

+        synchronized (Linker.class) {
+            if (sLoadedLibraries == null) {
+                if (DEBUG) Log.i(TAG, "No libraries loaded");
+            } else {
+                if (sCreateSharedRelros) {
+                    // Create new Bundle containing RELRO section information
+                    // for all loaded libraries. Make it available to getSharedRelros().
+                    sSharedRelros = createBundleFromLibInfoMap(sLoadedLibraries);
+                    if (DEBUG) {
+                        Log.i(TAG, "Shared RELRO created");
+                        dumpBundle(sSharedRelros);
+                    }
+                }
+
+                if (sWantSharedRelros) {
+                    // Wait until the shared relro bundle is received from useSharedRelros().
+                    while (sSharedRelros == null) {
+                        try {
+                            Linker.class.wait();
+                        } catch (InterruptedException ie) {
+                        }
+                    }
+                    useSharedRelrosLocked(sSharedRelros);
+                    // Clear the Bundle to ensure its file descriptor references can't be reused.
+                    sSharedRelros.clear();
+                    sSharedRelros = null;
+                }
+            }
+        }
+        if (DEBUG) Log.i(TAG, "finishLibraryLoad() exiting");
+     }
+
+    /**
+     * Call this to send a Bundle containing the shared RELRO sections to be
+     * used in this process. If wantSharedRelros() was previously called,
+     * finishLibraryLoad() will not exit until this method is called in another
+     * thread with a non-null value.
+     * @param bundle The Bundle instance containing a map of shared RELRO sections
+     * to use in this process.
+     */
+    public static void useSharedRelros(Bundle bundle) {
+        // Ensure the bundle uses the application's class loader, not the framework
+        // one which doesn't know anything about LibInfo.
+        if (bundle != null)
+            bundle.setClassLoader(LibInfo.class.getClassLoader());
+
+        if (DEBUG) Log.i(TAG, "useSharedRelros() called with " + bundle);
+
+        synchronized (Linker.class) {
+            // Note that in certain cases, this can be called before
+            // wantSharedRelros() in service processes.
+            sSharedRelros = bundle;
+            // Tell any listener blocked in finishLibraryLoad() about it.
+            Linker.class.notifyAll();
+        }
+    }
+
+    /**
+     * Call this to retrieve the shared RELRO sections created in this process,
+     * after loading all libraries.
+     * @return a new Bundle instance, or null if RELRO sharing is disabled on
+     * this system, or if wantSharedRelros() was called previously.
+     */
+    public static Bundle getSharedRelros() {
+        if (DEBUG) Log.i(TAG, "getSharedRelros() called");
+        synchronized (Linker.class) {
+            if (!sCreateSharedRelros)
+                return null;
+
+            // Return the Bundle created in finishLibraryLoad().
+            return sSharedRelros;
+        }
+    }
+
+
+    /**
+     * Call this method before loading any libraries to indicate that this
+     * process shall neither create or reuse shared RELRO sections.
+     */
+    public static void disableSharedRelros() {
+        if (DEBUG) Log.i(TAG, "disableSharedRelros() called");
+        synchronized (Linker.class) {
+            sCreateSharedRelros = false;
+            sWantSharedRelros = false;
+        }
+    }
+
+    /**
+     * Call this method before loading any libraries to indicate that this
+     * process is ready to reuse shared RELRO sections from another one.
+     * Typically used when starting service processes.
+     * @param baseLoadAddress the base library load address to use.
+     */
+    public static void wantSharedRelros(long baseLoadAddress) {
+        if (DEBUG) Log.i(TAG, String.format("wantSharedRelros(0x%x) called", baseLoadAddress));
+        synchronized (Linker.class) {
+            ensureInitializedLocked();
+            if (sRelroSharingSupported) {
+                sCreateSharedRelros = false;
+                sWantSharedRelros = true;
+                sBaseLoadAddress = baseLoadAddress;
+                sCurrentLoadAddress = baseLoadAddress;
+            }
+        }
+    }
+
+    /**
+     * Retrieve the base load address of all shared RELRO sections.
+     * This also enforces the creation of shared RELRO sections in
+     * prepareLibraryLoad(), which can later be retrieved with getSharedRelros().
+     * @return a common, random base load address, or 0 if RELRO sharing is
+     * disabled.
+     */
+    public static long getBaseLoadAddress() {
+        synchronized (Linker.class) {
+            ensureInitializedLocked();
+            if (!sCreateSharedRelros) {
+                Log.w(TAG, "Shared RELRO sections are disabled in this process!");
+                return 0;
+            }
+
+            setupBaseLoadAddressLocked();
+            if (DEBUG) Log.i(TAG, String.format("getBaseLoadAddress() returns 0x%x",
+                                                sBaseLoadAddress));
+            return sBaseLoadAddress;
+        }
+    }
+
+    // Used internally to lazily setup the common random base load address.

[palmer, 2013/09/18 21:56:36]

As with the other method that assumes its callers have acquired the lock, I'd
document it (and preferably enforce it) here, too.

[digit1, 2013/09/19 17:23:50]

assert added. Thanks.

+    private static void setupBaseLoadAddressLocked() {
+        if (sBaseLoadAddress == 0) {
+            long address = computeRandomBaseLoadAddress();
+            sBaseLoadAddress = address;
+            sCurrentLoadAddress = address;
+        }
+    }
+
+
+    /**
+     * Compute a random base load address where to place loaded libraries.
+     * @return new base load address, or 0 if the system does not support
+     * RELRO sharing.
+     */
+    private static long computeRandomBaseLoadAddress() {
+        Random r = new Random();

[palmer, 2013/09/18 19:51:11]

This is not random for the purposes of security. We need SecureRandom (and,
since that is broken, we need to explicitly seed it with 16 bytes from
/dev/urandom. Or, just create a long by reading directly from urandom).

[palmer, 2013/09/18 21:56:36]

Sorry, wrote that in a hurry. SecureRandom is documented as the "secure" way to
get random values that attackers cannot predict or learn; Random is explicitly
*not* that (and is correctly documented to be predictable).

But, we have recently learned that Android's SecureRandom had a weak
implementation (it's fixed now, but the fix is not shipped). So, we need to
ensure that the SecureRandom generator is seeded with truly unpredictable
values, such as some bytes from /dev/urandom. It might even be more
straightforward to just generate the random (pointer-sized) int you want
directly from bytes from /dev/urandom.

(Honestly, everything wrapping a simple read from urandom is likely to be a
vulnerability factory, and in any case more complicated.)

[digit1, 2013/09/19 17:23:50]

I'm not sure I fully understand your last paragraph. Do you mean that it's
better to read directly from /dev/urandom that using a helper class like
SecureRandom() which does who knows what? If so, I've implemented reading from
/dev/urandom here.

+
+        // The kernel ASLR feature will place randomized mappings starting
+        // from this address. Never try to load anything above this
+        // explicitely to avoid random conflicts.
+        final long baseAddressLimit = 0x40000000;
+
+        // Start loading libraries from this base address.
+        final long baseAddress = 0x20000000;
+
+        // Maximum randomized base address value. Used to ensure a margin
+        // of 192 MB below baseAddressLimit.
+        final long baseAddressMax = baseAddressLimit - 192 * 1024 * 1024;
+
+        // There is no way to get this from Java APIs, and it's not
+        // possible to call JNI here since no shared library has been
+        // loaded yet. All current Android platforms have a 4 KB page
+        // size at the moment. Correct this in the future when this is
+        // no longer true.
+        final long pageSize = 4096;
+
+        // Get a proper random page-aligned offset.
+        final int offset = r.nextInt((int)((baseAddressMax - baseAddress) / pageSize));
+
+        final long address = baseAddress + offset * pageSize;
+
+        if (DEBUG) {
+            Log.i(TAG,
+                  String.format("Linker.computeRandomBaseLoadAddress() return 0x%x",
+                                address));
+        }
+        return address;
+    }
+
+    // Used for debugging only.

[palmer, 2013/09/18 21:56:36]

Might be nice to wrap the body of this method in an if (DEBUG) { ... }. The
compiler might even be able to optimize it away when |DEBUG| is false.

[digit1, 2013/09/19 17:23:50]

I'm relying on proguard removing unused private methods from the final .class,
which happens here if DEBUG is not defined, but your suggestion is valid and
more explicit, so I've implemented it.

+    private static void dumpBundle(Bundle bundle) {
+        Log.i(TAG, "Bundle has " + bundle.size() + " items: " + bundle.toString());
+    }
+
+    /**
+     * Use the shared RELRO section from a Bundle received form another process.
+     * Call this after calling setBaseLoadAddress() then loading all libraries
+     * with loadLibrary().
+     * @param a Bundle instance generated with createSharedRelroBundle() in
+     * another process.
+     */
+    private static void useSharedRelrosLocked(Bundle bundle) {
+        if (DEBUG) Log.i(TAG, "Linker.useSharedRelrosLocked() called");
+
+        if (bundle == null) {
+            if (DEBUG) Log.i(TAG, "null bundle!");
+            return;
+        }
+
+        if (!sRelroSharingSupported) {
+            if (DEBUG) Log.i(TAG, "System does not support RELRO sharing");
+            return;
+        }
+
+        if (sLoadedLibraries == null) {
+            if (DEBUG) Log.i(TAG, "No libraries loaded!");
+            return;
+        }
+
+        if (DEBUG) dumpBundle(bundle);
+        HashMap<String, LibInfo> relroMap = createLibInfoMapFromBundle(bundle);
+        if (relroMap == null) {
+            if (DEBUG) Log.i(TAG, "Empty RELRO section map!?");
+            return;
+        }
+
+        // Apply the RELRO section to all libraries that were already loaded.
+        for (Map.Entry<String, LibInfo> entry : relroMap.entrySet()) {
+            String libName = entry.getKey();
+            LibInfo libInfo = entry.getValue();
+            if (!nativeUseSharedRelro(libName, libInfo)) {
+                Log.w(TAG, "Could not use shared RELRO section for " + libName);
+            } else {
+                if (DEBUG) Log.i(TAG, "Using shared RELRO section for " + libName);
+            }
+        }
+
+        // Close all file descriptors from the map now.
+        closeLibInfoMap(relroMap);
+
+        if (DEBUG) Log.i(TAG, "Linker.useSharedRelrosLocked() exiting");
+    }
+
+    /**
+     * Returns whether the linker was unable to load one library at a given fixed address.
+     *
+     * @return true if at least one library was not loaded at the expected fixed address.
+     */
+    public static boolean loadAtFixedAddressFailed() {
+        return sLoadAtFixedAddressFailed;
+    }
+
+    /**
+     * Load a native shared library with the Chromium linker.
+     * If neither initSharedRelro() or readFromBundle() were called
+     * previously, this uses the standard linker (i.e. System.loadLibrary()).
+     *
+     * @param library The library's base name.
+     * @throws UnsatisfiedLinkError if the library does not exist.
+     */
+    public static void loadLibrary(String library) {
+        if (DEBUG) Log.i(TAG, "loadLibrary: " + library);
+
+        // Security: RELRO sections should never be used in the same process
+        // that creates them.
+        assert !(sCreateSharedRelros && sWantSharedRelros);
+
+        // Don't self-load the linker. This is because the build system is
+        // not clever enough to understand that all the libraries packaged
+        // in the final .apk don't need to be explicitely loaded.
+        if (library.startsWith(TAG)) {
+            if (DEBUG) Log.i(TAG, "ignoring self-linker load");
+            return;
+        }
+
+        synchronized (Linker.class) {
+            ensureInitializedLocked();
+

[palmer, 2013/09/18 21:56:36]

Nit: Only one or zero blank lines.

[digit1, 2013/09/19 17:23:50]

Done.

+
+            String libName = System.mapLibraryName(library);
+
+            if (sLoadedLibraries == null)
+              sLoadedLibraries = new HashMap<String, LibInfo>();
+
+            if (sLoadedLibraries.containsKey(libName)) {
+                if (DEBUG) Log.i(TAG, "Not loading " + libName + " twice");
+                return;
+            }
+
+            LibInfo libInfo = new LibInfo();
+            LibInfo relroLibInfo = null;
+            long loadAddress = 0;
+            if (sWantSharedRelros) {
+                // If sWantSharedRelros is true, then always load libraries at explicit addresses.
+                loadAddress = sCurrentLoadAddress;
+            }
+
+            if (!nativeLoadLibrary(libName, loadAddress, libInfo)) {
+                Log.e(TAG, "Unable to load library: " + libName);

[palmer, 2013/09/18 21:56:36]

Nit: I'd construct the string only once, (a) to ensure the log message is the
same as the exception message, to avoid confusion; and (b) to avoid creating
more Strings than necessary.

[digit1, 2013/09/19 17:23:50]

Done.

+                throw new UnsatisfiedLinkError("Unable to load library: " + libName);
+            }
+            // Keep track whether the library has been loaded at the expected load address.
+            if (loadAddress != 0 && loadAddress != libInfo.mLoadAddress)
+                sLoadAtFixedAddressFailed = true;
+
+            if (sCreateSharedRelros) {
+                // Create a new shared RELRO section at the current load address.
+                if (!nativeCreateSharedRelro(libName, sCurrentLoadAddress, libInfo)) {
+                  Log.w(TAG,
+                      String.format(
+                          "Could not create shared RELRO for %s at %x",
+                          libName, sCurrentLoadAddress));
+                } else {
+                    if (DEBUG) Log.i(TAG,
+                        String.format(
+                            "Created shared RELRO for %s at %x: %s",
+                            libName,
+                            sCurrentLoadAddress,
+                            libInfo.toString()));
+                }
+            }
+
+            if (sCurrentLoadAddress != 0) {
+                // Compute the next current load address. If sBaseLoadAddress
+                // is not 0, this is an explicit library load address. Otherwise,
+                // this is an explicit load address for relocated RELRO sections
+                // only.
+                sCurrentLoadAddress = libInfo.mLoadAddress + libInfo.mLoadSize;
+            }
+
+            sLoadedLibraries.put(libName, libInfo);
+            if (DEBUG) Log.i(TAG, "Library details " + libInfo.toString());

[palmer, 2013/09/18 21:56:36]

Nit: Another thought on this code pattern: If you want to avoid the cost of a
call to Log.foo when |DEBUG| is false --- I assume that's why you wrap them in
checks against |DEBUG| --- you can take advantage of the private static
final-ness of DEBUG and do:

private static void logDebug(String tag, String message) {
    if (DEBUG) {
        Log.d(tag, message);
    }
}

and then all those calls will not only be cheap, they will (likely) be compiled
away. And then the call sites will look cleaner, too.

[digit1, 2013/09/19 17:23:50]

I don't know where you learnt about this technique, but it is _definitely_
wrong.

The Java compiler does optimizes constant final values, which means that if
DEBUG is defined as false, then a statement like:

   if (DEBUG) doSomethingCostly();

Does not get compiled in, that why all calls to Log.i() here use this idiom.

On the other hand, with your proposal, what will happen is that:

  - logDebug() will be compiled as an empty function.
  - any call to logDebug() will still be compiled in, including any costly
operation needed to craft its parameters

In other words, the compiler is not capable of optimizing out calls to empty
private static functions. Proguard won't do it either.

That's a major pet peeve of the Java language and why the "if (DEBUG)
doCostlyStuff();" idiom must be used everywhere you need that kind of
conditional logging. Again, this is used all over the place on both the Android
framework and Chrome for Android sources.

+        }
+    }
+
+    /**
+     * Native method used to load a library.
+     * @param library Platform specific library name (e.g. libfoo.so)
+     * @param loadAddress Explicit load address, or 0 for randomized one.
+     * @param relro_info If not null, the mLoadAddress and mLoadSize fields
+     * of this LibInfo instance will set on success.
+     * @return true for success, false otherwise.
+     */
+    private static native boolean nativeLoadLibrary(String library,
+                                                    long  loadAddress,
+                                                    LibInfo libInfo);
+
+    /**
+     * Native method used to create a shared RELRO section.
+     * If the library was already loaded at the same address using
+     * nativeLoadLibrary(), this creates the RELRO for it. Otherwise,
+     * this loads a new temporary library at the specified address,
+     * creates and extracts the RELRO section from it, then unloads it.
+     * @param library Library name.
+     * @param loadAddress load address, which can be different from the one
+     * used to load the library in the current process!
+     * @param LibInfo libInfo instance. On success, the mRelroStart, mRelroSize
+     * and mRelroFd will be set.
+     * @return true on success, false otherwise.
+     */
+    private static native boolean nativeCreateSharedRelro(String library,
+                                                          long loadAddress,
+                                                          LibInfo libInfo);
+
+    /**
+     * Native method used to use a shared RELRO section.
+     * @param library Library name.
+     * @param libInfo A LibInfo instance containing valid RELRO information
+     * @return true on success.
+     */
+    private static native boolean nativeUseSharedRelro(String library,
+                                                       LibInfo libInfo);
+
+    /**
+     * Checks that the system supports shared RELROs. Old Android kernels
+     * have a bug in the way they check Ashmem region protection flags, which
+     * makes using shared RELROs unsafe. This method performs a simple runtime
+     * check for this misfeature, even though nativeEnableSharedRelro() will
+     * always fail if this returns false.
+     */
+    private static native boolean nativeCanUseSharedRelro();
+
+    /**
+     * Record information for a given library.
+     * IMPORTANT: Native code knows anout this class's fields, so

[palmer, 2013/09/18 21:56:36]

Typo: "about"

[digit1, 2013/09/19 17:23:50]

Done.

+     * don't change them without modifying the corresponding C++ sources.
+     * Also, the LibInfo instance owns the ashmem file descriptor.
+     */
+    public static class LibInfo implements Parcelable {
+
+        public LibInfo() {
+            mLoadAddress = 0;
+            mLoadSize = 0;
+            mRelroStart = 0;
+            mRelroSize = 0;
+            mRelroFd = -1;
+        }
+
+        public void close() {
+            if (mRelroFd >= 0) {
+              try {
+                  ParcelFileDescriptor.adoptFd(mRelroFd).close();
+              } catch (java.io.IOException e) {
+              }
+              mRelroFd = -1;
+            }
+        }
+
+        // from Parcelable
+        public LibInfo(Parcel in) {
+            mLoadAddress = in.readLong();
+            mLoadSize = in.readLong();
+            mRelroStart = in.readLong();
+            mRelroSize = in.readLong();
+            ParcelFileDescriptor fd = in.readFileDescriptor();
+            mRelroFd = fd.detachFd();
+        }
+
+        // from Parcelable
+        @Override
+        public void writeToParcel(Parcel out, int flags) {
+            if (mRelroFd >= 0) {
+                out.writeLong(mLoadAddress);
+                out.writeLong(mLoadSize);
+                out.writeLong(mRelroStart);
+                out.writeLong(mRelroSize);
+                try {
+                    ParcelFileDescriptor fd = ParcelFileDescriptor.fromFd(mRelroFd);
+                    fd.writeToParcel(out, 0);
+                    fd.close();
+                } catch (java.io.IOException e) {
+                    Log.e(TAG, "Cant' write LibInfo file descriptor to parcel", e);
+                }
+            }
+        }
+
+        // from Parcelable
+        @Override
+        public int describeContents() {
+            return Parcelable.CONTENTS_FILE_DESCRIPTOR;
+        }
+
+        // from Parcelable
+        public static final Parcelable.Creator<LibInfo> CREATOR
+                = new Parcelable.Creator<LibInfo>() {
+            public LibInfo createFromParcel(Parcel in) {
+                return new LibInfo(in);
+            }
+
+            public LibInfo[] newArray(int size) {
+                return new LibInfo[size];
+            }
+        };
+
+        public String toString() {
+            return String.format("[load=0x%x-0x%x relro=0x%x-0x%x fd=%d]",
+                                 mLoadAddress,
+                                 mLoadAddress + mLoadSize,
+                                 mRelroStart,
+                                 mRelroStart + mRelroSize,
+                                 mRelroFd);
+        }
+
+        // IMPORTANT: Don't change these fields without modifying the
+        // native code that accesses them directly!
+        public long mLoadAddress; // page-aligned library load address.
+        public long mLoadSize;    // page-aligned library load size.
+        public long mRelroStart;  // page-aligned address in memory, or 0 if none.
+        public long mRelroSize;   // page-aligned size in memory, or 0.
+        public int  mRelroFd;     // ashmem file descriptor, or -1
+    }
+
+    // Create a Bundle from a map of LibInfo objects.
+    private static Bundle createBundleFromLibInfoMap(HashMap<String, LibInfo> map) {
+        Bundle bundle = new Bundle(map.size());
+        for (Map.Entry<String, LibInfo> entry : map.entrySet())
+            bundle.putParcelable(entry.getKey(), entry.getValue());
+
+        return bundle;
+    }
+
+    // Create a new LibInfo map from a Bundle.
+    private static HashMap<String, LibInfo> createLibInfoMapFromBundle(Bundle bundle) {
+        HashMap<String, LibInfo> map = new HashMap<String, LibInfo>();
+        for (String library : bundle.keySet()) {
+          LibInfo libInfo = bundle.getParcelable(library);
+          map.put(library, libInfo);
+        }
+        return map;
+    }
+
+    // Call the close() method on all values of a LibInfo map.
+    private static void closeLibInfoMap(HashMap<String, LibInfo> map) {
+        for (Map.Entry<String, LibInfo> entry : map.entrySet())
+            entry.getValue().close();
+    }
+
+    // The map of libraries that are currently loaded in this process.
+    private static HashMap<String, LibInfo> sLoadedLibraries = null;
+
+    // Used to pass the shared RELRO Bundle through Binder.
+    public static final String EXTRA_LINKER_SHARED_RELROS =
+        "org.chromium.content.common.linker.shared_relros";
+}