base/android: Make Linker an implementation detail of LibraryLoader

Restrict Linker functionality to be "package private", do not expose
the whole Linker API to clients.

Reduce verbosity in code comments about the Linker, and reformulate them
in terms of RELRO producer/consumer, rather than process type. This way
the App Zygote can become the RELRO producer without contradicting all
these explanations.

Remove ChromiumLinkerParams. Previously I thought that in the new
protocol it would expand, but now I realized that it is better to extend
the messages that happen later in Linker lifetime.

Extract the bundle exchange between linkers in different processes into
LibraryLoader's inner class. This looks cleaner because it makes the
communication protocol handled in one compact place. Before this change
it was nontrivial to verify that communication is not racy.

The plan is to add functionality to the MultiProcessMessageHandler to
make it possible to sometimes move the RELRO FD from isolated processes
into the browser process, without leaking a lot of knowledge about it
outside of the package o.c.b.library_loader. With this change the
MultiProcessMessageHandler will make sense to move to another file.

Bug: 1154224
Change-Id: I2c027961530947a93bca44ce70b8b9dcd25e1b2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2607931
Commit-Queue: Egor Pasko <pasko@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840993}

base/android/java/src/org/chromium/base/library_loader/LegacyLinker.java

@@ -37,7 +37,7 @@ class LegacyLinker extends Linker {
         ensureInitializedLocked();
         assert mState == State.INITIALIZED; // Only one successful call.
 
-        boolean provideRelro = mInBrowserProcess;
+        boolean produceRelro = mRelroProducer;
         long loadAddress = isFixedAddressPermitted ? mBaseLoadAddress : 0;
 
         String libFilePath = System.mapLibraryName(library);
@@ -50,7 +50,7 @@ class LegacyLinker extends Linker {
         }
         libInfo.mLibFilePath = libFilePath;
 
-        if (provideRelro) {
+        if (produceRelro) {
             if (!nativeCreateSharedRelro(sharedRelRoName, mBaseLoadAddress, libInfo)) {
                 Log.w(TAG, "Could not create shared RELRO for %s at %x", libFilePath,
                         mBaseLoadAddress);
@@ -67,6 +67,7 @@ class LegacyLinker extends Linker {
             useSharedRelrosLocked(mLibInfo);
             mState = State.DONE_PROVIDE_RELRO;
         } else {
+            // Consume RELRO.
             waitForSharedRelrosLocked();
             assert libFilePath.equals(mLibInfo.mLibFilePath);
             useSharedRelrosLocked(mLibInfo);

base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java

@@ -10,6 +10,7 @@ import android.content.SharedPreferences;
 import android.content.pm.ApplicationInfo;
 import android.os.Build;
 import android.os.Build.VERSION_CODES;
+import android.os.Bundle;
 import android.os.SystemClock;
 import android.system.Os;
 
@@ -40,6 +41,7 @@ import java.lang.annotation.RetentionPolicy;
 import java.util.Locale;
 
 import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
 
 /**
  * This class provides functionality to load and register the native libraries.
@@ -101,7 +103,7 @@ public class LibraryLoader {
     // Avoids locking: should be initialized very early.
     private boolean mConfigurationSet;
 
-    // The type of process the shared library is loaded in.
+    // The type of process the shared library is loaded in. Gets passed to native after loading.
     // Avoids locking: should be initialized very early.
     private @LibraryProcessType int mLibraryProcessType;
 
@@ -109,6 +111,9 @@ public class LibraryLoader {
     // except the volatile |mLoadState|.
     private final Object mNonMainDexLock = new Object();
 
+    // Mediates all communication between Linker instances in different processes.
+    private final MultiProcessMediator mMessageHandler = new MultiProcessMediator();
+
     // Guards all the fields below.
     private final Object mLock = new Object();
 
@@ -128,11 +133,118 @@ public class LibraryLoader {
     @GuardedBy("mLock")
     private boolean mCommandLineSwitched;
 
-    // The number of milliseconds it took to load all the native libraries, which
-    // will be reported via UMA. Set once when the libraries are done loading.
+    // The number of milliseconds it took to load all the native libraries, which will be reported
+    // via UMA. Set once when the libraries are done loading.
     @GuardedBy("mLock")
     private long mLibraryLoadTimeMs;
 
+    /**
+     * Inner class encapsulating points of communication between instances of LibraryLoader in
+     * different processes.
+     *
+     * Usage:
+     *
+     * - For a {@link LibraryLoader} requiring the knowledge of the load address before
+     *   initialization, {@link #takeLoadAddressFromBundle(Bundle)} should be called first. It is
+     *   done very early after establishing a Binder connection.
+     *
+     * - To initialize the object, one of {@link #ensureInitializedInMainProcess()} and
+     *   {@link #initInChildProcess()} must be called. Subsequent calls to initialization are
+     *   ignored.
+     *
+     * - Later  {@link #putLoadAddressToBundle(Bundle)} and
+     *   {@link #takeLoadAddressFromBundle(Bundle)} should be called for passing the RELRO
+     *   information between library loaders.
+     *
+     * Internally the {@LibraryLoader} may ignore these messages because it can fall back to not
+     * sharing RELRO.
+     */
+    @ThreadSafe
+    public class MultiProcessMediator {
+        @GuardedBy("mLock")
+        private long mLoadAddress;
+
+        // Used only for asserts, and only ever switched from false to true.
+        private volatile boolean mInitDone;
+
+        /**
+         * Extracts the load address as provided by another process.
+         * @param bundle The Bundle to extract from.
+         */
+        public void takeLoadAddressFromBundle(Bundle bundle) {
+            // Currently clients call this method strictly before any other method can get executed
+            // on a different thread. Hence, synchronization is not required, but verification of
+            // correctness is still non-trivial, and over-synchronization is cheap compared to
+            // library loading.
+            synchronized (mLock) {
+                mLoadAddress = Linker.extractLoadAddressFromBundle(bundle);
+            }
+        }
+
+        /**
+         * Initializes the Browser process side of communication, the one that coordinates creation
+         * of other processes. Can be called more than once, subsequent calls are ignored.
+         */
+        public void ensureInitializedInMainProcess() {
+            if (mInitDone) return;
+            if (useChromiumLinker()) {
+                Linker.getInstance().initAsRelroProducer();
+            }
+            mInitDone = true;
+        }
+
+        /**
+         * Serializes the load address for communication, if any was determined during
+         * initialization. Must be called after the library has been loaded in this process.
+         * @param bundle Bundle to put the address to.
+         */
+        public void putLoadAddressToBundle(Bundle bundle) {
+            assert mInitDone;
+            if (useChromiumLinker()) {
+                Linker.getInstance().putLoadAddressToBundle(bundle);
+            }
+        }
+
+        /**
+         * Initializes in processes other than "Main".
+         */
+        public void initInChildProcess() {
+            if (useChromiumLinker()) {
+                synchronized (mLock) {
+                    Linker.getInstance().initAsRelroConsumer(mLoadAddress);
+                }
+            }
+            mInitDone = true;
+        }
+
+        /**
+         * Optionally extracts RELRO and saves it for replacing the RELRO section in this process.
+         * Can be invoked before initialization.
+         * @param bundle Where to deserialize from.
+         */
+        public void takeSharedRelrosFromBundle(Bundle bundle) {
+            if (useChromiumLinker() && !isLoadedByZygote()) {
+                Linker.getInstance().takeSharedRelrosFromBundle(bundle);
+            }
+        }
+
+        /**
+         * Optionally puts the RELRO section information so that it can be memory-mapped in another
+         * process reading the bundle.
+         * @param bundle Where to serialize.
+         */
+        public void putSharedRelrosToBundle(Bundle bundle) {
+            assert mInitDone;
+            if (useChromiumLinker()) {
+                Linker.getInstance().putSharedRelrosToBundle(bundle);
+            }
+        }
+    }
+
+    public final MultiProcessMediator getMediator() {
+        return mMessageHandler;
+    }
+
     /**
      * Call this method to determine if the chromium project must load the library
      * directly from a zip file.
@@ -189,8 +301,9 @@ public class LibraryLoader {
      * Must be called before loading the library. Since this function is called extremely early on
      * in startup, locking is not required.
      *
-     * @param useChromiumLinker Whether to use the chromium linker.
-     * @param useModernLinker Whether to use ModernLinker.
+     * @param useChromiumLinker Whether to use a chromium linker.
+     * @param useModernLinker Given that one of the Chromium linkers is used, whether to use
+     *                        ModernLinker instea of the LegacyLinker.
      */
     public void setLinkerImplementation(boolean useChromiumLinker, boolean useModernLinker) {
         assert !mInitialized;
@@ -234,7 +347,7 @@ public class LibraryLoader {
         return result;
     }
 
-    public boolean useChromiumLinker() {
+    private boolean useChromiumLinker() {
         return mUseChromiumLinker && !forceSystemLinker();
     }
 
@@ -329,7 +442,7 @@ public class LibraryLoader {
     }
 
     /**
-     * Checks if library is fully loaded and initialized.
+     * Checks whether the native library is fully loaded and initialized.
      */
     public boolean isInitialized() {
         return mInitialized && mLoadState == LoadState.LOADED;
@@ -372,9 +485,9 @@ public class LibraryLoader {
     }
 
     /**
-     * Initializes the library here and now: must be called on the thread that the
+     * Initializes the native library: must be called on the thread that the
      * native will call its "main" thread. The library must have previously been
-     * loaded with loadNow.
+     * loaded with one of the loadNow*() variants.
      */
     public void initialize() {
         synchronized (mLock) {

base/android/java/src/org/chromium/base/library_loader/ModernLinker.java

@@ -38,7 +38,7 @@ class ModernLinker extends Linker {
 
         String libFilePath = System.mapLibraryName(library);
         boolean loadNoRelro = !isFixedAddressPermitted;
-        boolean provideRelro = isFixedAddressPermitted && mInBrowserProcess;
+        boolean provideRelro = isFixedAddressPermitted && mRelroProducer;
         long loadAddress = isFixedAddressPermitted ? mBaseLoadAddress : 0;
 
         if (loadNoRelro) {
@@ -60,7 +60,7 @@ class ModernLinker extends Linker {
                 libInfo.mRelroFd = -1;
             }
             mLibInfo = libInfo;
-            Log.d(TAG, "Successfully spawned RELRO: mLoadAddress=%d, mLoadSize=%d",
+            Log.d(TAG, "Successfully spawned RELRO: mLoadAddress=0x%x, mLoadSize=%d",
                     mLibInfo.mLoadAddress, mLibInfo.mLoadSize);
             // Next state is still to provide relro (even if we don't have any), as child processes
             // would wait for them.
@@ -69,7 +69,7 @@ class ModernLinker extends Linker {
             // Running in a child process, also with a fixed load address that is suitable for
             // shared RELRO.
             waitForSharedRelrosLocked();
-            Log.d(TAG, "Received mLibInfo: mLoadAddress=%d, mLoadSize=%d", mLibInfo.mLoadAddress,
+            Log.d(TAG, "Received mLibInfo: mLoadAddress=0x%x, mLoadSize=%d", mLibInfo.mLoadAddress,
                     mLibInfo.mLoadSize);
             // Two LibInfo objects are used: |mLibInfo| that brings the RELRO FD, and a temporary
             // LibInfo to load the library. Before replacing the library's RELRO with the one from

content/public/android/BUILD.gn

@@ -137,7 +137,6 @@ android_library("content_java") {
   ]
 
   sources = [
-    "java/src/org/chromium/content/app/ChromiumLinkerParams.java",
     "java/src/org/chromium/content/app/ContentChildProcessService.java",
     "java/src/org/chromium/content/app/ContentChildProcessServiceDelegate.java",
     "java/src/org/chromium/content/app/ContentMain.java",

content/public/android/java/src/org/chromium/content/app/ChromiumLinkerParams.java

-// Copyright 2014 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.app;
-
-import android.os.Bundle;
-
-import java.util.Locale;
-
-import javax.annotation.concurrent.Immutable;
-
-/**
- * A class to hold information passed from the browser process to each
- * service one when using the chromium linker. For more information, read the
- * technical notes in Linker.java.
- */
-@Immutable
-public class ChromiumLinkerParams {
-    // Use this base address to load native shared libraries. If 0, ignore other members.
-    public final long mBaseLoadAddress;
-
-    private static final String EXTRA_LINKER_PARAMS_BASE_LOAD_ADDRESS =
-            "org.chromium.content.common.linker_params.base_load_address";
-
-    public ChromiumLinkerParams(long baseLoadAddress) {
-        mBaseLoadAddress = baseLoadAddress;
-    }
-
-    /**
-     * Use this method to recreate a LinkerParams instance from a Bundle.
-     *
-     * @param bundle A Bundle, its content must have been populated by a previous
-     * call to populateBundle().
-     * @return params instance or possibly null if params was not put into bundle.
-     */
-    public static ChromiumLinkerParams create(Bundle bundle) {
-        if (!bundle.containsKey(EXTRA_LINKER_PARAMS_BASE_LOAD_ADDRESS)) return null;
-        return new ChromiumLinkerParams(bundle);
-    }
-
-    private ChromiumLinkerParams(Bundle bundle) {
-        mBaseLoadAddress = bundle.getLong(EXTRA_LINKER_PARAMS_BASE_LOAD_ADDRESS, 0);
-    }
-
-    /**
-     * Save data in this LinkerParams instance in a bundle, to be sent to a service process.
-     *
-     * @param bundle An bundle to be passed to the child service process.
-     */
-    public void populateBundle(Bundle bundle) {
-        bundle.putLong(EXTRA_LINKER_PARAMS_BASE_LOAD_ADDRESS, mBaseLoadAddress);
-    }
-
-    // For debugging traces only.
-    @Override
-    public String toString() {
-        return String.format(Locale.US, "LinkerParams(baseLoadAddress:0x%x", mBaseLoadAddress);
-    }
-}

content/public/android/java/src/org/chromium/content/app/ContentChildProcessServiceDelegate.java

@@ -20,7 +20,6 @@ import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.MainDex;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.base.library_loader.LibraryLoader;
-import org.chromium.base.library_loader.Linker;
 import org.chromium.base.memory.MemoryPressureUma;
 import org.chromium.base.process_launcher.ChildProcessServiceDelegate;
 import org.chromium.base.task.PostTask;
@@ -34,17 +33,14 @@ import org.chromium.content_public.common.ContentProcessInfo;
 import java.util.List;
 
 /**
- * This implementation of {@link ChildProcessServiceDelegate} loads the native library potentially
- * using the custom linker, provides access to view surfaces.
+ * This implementation of {@link ChildProcessServiceDelegate} loads the native library, provides
+ * access to view surfaces.
  */
 @JNINamespace("content")
 @MainDex
 public class ContentChildProcessServiceDelegate implements ChildProcessServiceDelegate {
     private static final String TAG = "ContentCPSDelegate";
 
-    // Linker-specific parameters for this child process service.
-    private ChromiumLinkerParams mLinkerParams;
-
     private IGpuProcessCallback mGpuCallback;
 
     private int mCpuCount;
@@ -63,7 +59,7 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe
 
     @Override
     public void onServiceBound(Intent intent) {
-        mLinkerParams = ChromiumLinkerParams.create(intent.getExtras());
+        LibraryLoader.getInstance().getMediator().takeLoadAddressFromBundle(intent.getExtras());
         LibraryLoader.getInstance().setLibraryProcessType(
                 ChildProcessCreationParamsImpl.getLibraryProcessType(intent.getExtras()));
     }
@@ -78,11 +74,7 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe
         mCpuFeatures = connectionBundle.getLong(ContentChildProcessConstants.EXTRA_CPU_FEATURES);
         assert mCpuCount > 0;
 
-        if (LibraryLoader.getInstance().useChromiumLinker()
-                && !LibraryLoader.getInstance().isLoadedByZygote()) {
-            Bundle sharedRelros = connectionBundle.getBundle(Linker.EXTRA_LINKER_SHARED_RELROS);
-            if (sharedRelros != null) getLinker().provideSharedRelros(sharedRelros);
-        }
+        LibraryLoader.getInstance().getMediator().takeSharedRelrosFromBundle(connectionBundle);
     }
 
     @Override
@@ -102,12 +94,10 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe
 
         JNIUtils.enableSelectiveJniRegistration();
 
-        if (LibraryLoader.getInstance().useChromiumLinker()) {
-            assert mLinkerParams != null;
-            getLinker().initServiceProcess(mLinkerParams.mBaseLoadAddress);
-        }
-        LibraryLoader.getInstance().loadNowOverrideApplicationContext(hostContext);
-        LibraryLoader.getInstance().registerRendererProcessHistogram();
+        LibraryLoader libraryLoader = LibraryLoader.getInstance();
+        libraryLoader.getMediator().initInChildProcess();
+        libraryLoader.loadNowOverrideApplicationContext(hostContext);
+        libraryLoader.registerRendererProcessHistogram();
         initializeLibrary();
     }
 
@@ -140,11 +130,6 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe
         ContentMain.start(false);
     }
 
-    // Return a Linker instance. If testing, the Linker needs special setup.
-    private Linker getLinker() {
-        return Linker.getInstance();
-    }
-
     @CalledByNative
     private void setFileDescriptorsIdsToKeys(int[] ids, String[] keys) {
         assert ids.length == keys.length;

content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelperImpl.java

@@ -28,14 +28,13 @@ import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.base.library_loader.LibraryLoader;
-import org.chromium.base.library_loader.Linker;
+import org.chromium.base.library_loader.LibraryLoader.MultiProcessMediator;
 import org.chromium.base.process_launcher.ChildConnectionAllocator;
 import org.chromium.base.process_launcher.ChildProcessConnection;
 import org.chromium.base.process_launcher.ChildProcessConstants;
 import org.chromium.base.process_launcher.ChildProcessLauncher;
 import org.chromium.base.process_launcher.FileDescriptorInfo;
 import org.chromium.base.task.PostTask;
-import org.chromium.content.app.ChromiumLinkerParams;
 import org.chromium.content.app.SandboxedProcessService;
 import org.chromium.content.common.ContentSwitchUtils;
 import org.chromium.content_public.browser.ChildProcessImportance;
@@ -146,10 +145,8 @@ public final class ChildProcessLauncherHelperImpl {
                             ContentChildProcessConstants.EXTRA_CPU_COUNT, CpuFeatures.getCount());
                     connectionBundle.putLong(
                             ContentChildProcessConstants.EXTRA_CPU_FEATURES, CpuFeatures.getMask());
-                    if (LibraryLoader.getInstance().useChromiumLinker()) {
-                        connectionBundle.putBundle(Linker.EXTRA_LINKER_SHARED_RELROS,
-                                Linker.getInstance().getSharedRelros());
-                    }
+                    LibraryLoader.getInstance().getMediator().putSharedRelrosToBundle(
+                            connectionBundle);
                 }
 
                 @Override
@@ -638,36 +635,13 @@ public final class ChildProcessLauncherHelperImpl {
         }
     }
 
-    private static boolean sLinkerInitialized;
-    private static long sLinkerLoadAddress;
-    private static void initLinker() {
-        assert LauncherThread.runningOnLauncherThread();
-        if (sLinkerInitialized) return;
-        if (LibraryLoader.getInstance().useChromiumLinker()) {
-            sLinkerLoadAddress = Linker.getInstance().getBaseLoadAddress();
-            if (sLinkerLoadAddress == 0) {
-                Log.i(TAG, "Shared RELRO support disabled!");
-            }
-        }
-        sLinkerInitialized = true;
-    }
-
-    private static ChromiumLinkerParams getLinkerParamsForNewConnection() {
-        assert LauncherThread.runningOnLauncherThread();
-
-        initLinker();
-        assert sLinkerInitialized;
-        if (sLinkerLoadAddress == 0) return null;
-
-        return new ChromiumLinkerParams(sLinkerLoadAddress);
-    }
-
     private static Bundle populateServiceBundle(Bundle bundle) {
         ChildProcessCreationParamsImpl.addIntentExtras(bundle);
         bundle.putBoolean(ChildProcessConstants.EXTRA_BIND_TO_CALLER,
                 ChildProcessCreationParamsImpl.getBindToCallerCheck());
-        ChromiumLinkerParams linkerParams = getLinkerParamsForNewConnection();
-        if (linkerParams != null) linkerParams.populateBundle(bundle);
+        MultiProcessMediator m = LibraryLoader.getInstance().getMediator();
+        m.ensureInitializedInMainProcess();
+        m.putLoadAddressToBundle(bundle);
         return bundle;
     }