Handle the race condition when calling uncrypt services.We call uncrypt services to setup / clear bootloader control block (BCB)for scheduling tasks under recovery (applying OTAs, performing FDR).However, we cannot start multiple requests simultaneously. Because theyall use the same socket (/dev/socket/uncrypt) for communication and initdeletes the socket on service exits.This CL fixes the issue by a) adding synchronized blocks for the servicerequests; b) checking the availability of the socket before initiating anew one.Note that adding synchronized blocks to RecoverySystem doesn't help,because the calls could be made from different processes (e.g. priv-app,system_server).Bug: 31526152Test: FDR works while a priv-app keeps calling clear BCB.Change-Id: I95308989e849a9c98a9503ac509f2bc51ed3de19

diff --git a/core/java/android/os/IRecoverySystem.aidl b/core/java/android/os/IRecoverySystem.aidlindex 12830a4..c5ceecd 100644--- a/core/java/android/os/IRecoverySystem.aidl+++ b/core/java/android/os/IRecoverySystem.aidl
@@ -25,4 +25,5 @@     boolean uncrypt(in String packageFile, IRecoverySystemProgressListener listener);     boolean setupBcb(in String command);     boolean clearBcb();+    void rebootRecoveryWithCommand(in String command); }
diff --git a/core/java/android/os/RecoverySystem.java b/core/java/android/os/RecoverySystem.javaindex 028e30a..df7df64 100644--- a/core/java/android/os/RecoverySystem.java+++ b/core/java/android/os/RecoverySystem.java
@@ -663,28 +663,22 @@      * @throws IOException if something goes wrong.      */     private static void bootCommand(Context context, String... args) throws IOException {-        synchronized (sRequestLock) {-            LOG_FILE.delete();+        LOG_FILE.delete(); -            StringBuilder command = new StringBuilder();-            for (String arg : args) {-                if (!TextUtils.isEmpty(arg)) {-                    command.append(arg);-                    command.append("\n");-                }+        StringBuilder command = new StringBuilder();+        for (String arg : args) {+            if (!TextUtils.isEmpty(arg)) {+                command.append(arg);+                command.append("\n");             }--            // Write the command into BCB (bootloader control block).-            RecoverySystem rs = (RecoverySystem) context.getSystemService(-                    Context.RECOVERY_SERVICE);-            rs.setupBcb(command.toString());--            // Having set up the BCB, go ahead and reboot.-            PowerManager pm = (PowerManager) context.getSystemService(Context.POWER_SERVICE);-            pm.reboot(PowerManager.REBOOT_RECOVERY);--            throw new IOException("Reboot failed (no permissions?)");         }++        // Write the command into BCB (bootloader control block) and boot from+        // there. Will not return unless failed.+        RecoverySystem rs = (RecoverySystem) context.getSystemService(Context.RECOVERY_SERVICE);+        rs.rebootRecoveryWithCommand(command.toString());++        throw new IOException("Reboot failed (no permissions?)");     }      // Read last_install; then report time (in seconds) and I/O (in MiB) for@@ -879,6 +873,17 @@     }      /**+     * Talks to RecoverySystemService via Binder to set up the BCB command and+     * reboot into recovery accordingly.+     */+    private void rebootRecoveryWithCommand(String command) {+        try {+            mService.rebootRecoveryWithCommand(command);+        } catch (RemoteException ignored) {+        }+    }++    /**      * Internally, recovery treats each line of the command file as a separate      * argv, so we only need to protect against newlines and nulls.      */
diff --git a/services/core/java/com/android/server/RecoverySystemService.java b/services/core/java/com/android/server/RecoverySystemService.javaindex 276687f..3c8c699 100644--- a/services/core/java/com/android/server/RecoverySystemService.java+++ b/services/core/java/com/android/server/RecoverySystemService.java
@@ -21,6 +21,7 @@ import android.net.LocalSocketAddress; import android.os.IRecoverySystem; import android.os.IRecoverySystemProgressListener;+import android.os.PowerManager; import android.os.RecoverySystem; import android.os.RemoteException; import android.os.SystemProperties;@@ -50,8 +51,15 @@     // The socket at /dev/socket/uncrypt to communicate with uncrypt.     private static final String UNCRYPT_SOCKET = "uncrypt"; +    // The init services that communicate with /system/bin/uncrypt.+    private static final String INIT_SERVICE_UNCRYPT = "init.svc.uncrypt";+    private static final String INIT_SERVICE_SETUP_BCB = "init.svc.setup-bcb";+    private static final String INIT_SERVICE_CLEAR_BCB = "init.svc.clear-bcb";+     private static final int SOCKET_CONNECTION_MAX_RETRY = 30; +    private static final Object sRequestLock = new Object();+     private Context mContext;      public RecoverySystemService(Context context) {@@ -69,95 +77,155 @@         public boolean uncrypt(String filename, IRecoverySystemProgressListener listener) {             if (DEBUG) Slog.d(TAG, "uncrypt: " + filename); -            mContext.enforceCallingOrSelfPermission(android.Manifest.permission.RECOVERY, null);+            synchronized (sRequestLock) {+                mContext.enforceCallingOrSelfPermission(android.Manifest.permission.RECOVERY, null); -            // Write the filename into UNCRYPT_PACKAGE_FILE to be read by-            // uncrypt.-            RecoverySystem.UNCRYPT_PACKAGE_FILE.delete();--            try (FileWriter uncryptFile = new FileWriter(RecoverySystem.UNCRYPT_PACKAGE_FILE)) {-                uncryptFile.write(filename + "\n");-            } catch (IOException e) {-                Slog.e(TAG, "IOException when writing \"" + RecoverySystem.UNCRYPT_PACKAGE_FILE +-                        "\": ", e);-                return false;-            }--            // Trigger uncrypt via init.-            SystemProperties.set("ctl.start", "uncrypt");--            // Connect to the uncrypt service socket.-            LocalSocket socket = connectService();-            if (socket == null) {-                Slog.e(TAG, "Failed to connect to uncrypt socket");-                return false;-            }--            // Read the status from the socket.-            DataInputStream dis = null;-            DataOutputStream dos = null;-            try {-                dis = new DataInputStream(socket.getInputStream());-                dos = new DataOutputStream(socket.getOutputStream());-                int lastStatus = Integer.MIN_VALUE;-                while (true) {-                    int status = dis.readInt();-                    // Avoid flooding the log with the same message.-                    if (status == lastStatus && lastStatus != Integer.MIN_VALUE) {-                        continue;-                    }-                    lastStatus = status;--                    if (status >= 0 && status <= 100) {-                        // Update status-                        Slog.i(TAG, "uncrypt read status: " + status);-                        if (listener != null) {-                            try {-                                listener.onProgress(status);-                            } catch (RemoteException ignored) {-                                Slog.w(TAG, "RemoteException when posting progress");-                            }-                        }-                        if (status == 100) {-                            Slog.i(TAG, "uncrypt successfully finished.");-                            // Ack receipt of the final status code. uncrypt-                            // waits for the ack so the socket won't be-                            // destroyed before we receive the code.-                            dos.writeInt(0);-                            break;-                        }-                    } else {-                        // Error in /system/bin/uncrypt.-                        Slog.e(TAG, "uncrypt failed with status: " + status);-                        // Ack receipt of the final status code. uncrypt waits-                        // for the ack so the socket won't be destroyed before-                        // we receive the code.-                        dos.writeInt(0);-                        return false;-                    }+                final boolean available = checkAndWaitForUncryptService();+                if (!available) {+                    Slog.e(TAG, "uncrypt service is unavailable.");+                    return false;                 }-            } catch (IOException e) {-                Slog.e(TAG, "IOException when reading status: ", e);-                return false;-            } finally {-                IoUtils.closeQuietly(dis);-                IoUtils.closeQuietly(dos);-                IoUtils.closeQuietly(socket);-            } -            return true;+                // Write the filename into UNCRYPT_PACKAGE_FILE to be read by+                // uncrypt.+                RecoverySystem.UNCRYPT_PACKAGE_FILE.delete();++                try (FileWriter uncryptFile = new FileWriter(RecoverySystem.UNCRYPT_PACKAGE_FILE)) {+                    uncryptFile.write(filename + "\n");+                } catch (IOException e) {+                    Slog.e(TAG, "IOException when writing \"" ++                            RecoverySystem.UNCRYPT_PACKAGE_FILE + "\":", e);+                    return false;+                }++                // Trigger uncrypt via init.+                SystemProperties.set("ctl.start", "uncrypt");++                // Connect to the uncrypt service socket.+                LocalSocket socket = connectService();+                if (socket == null) {+                    Slog.e(TAG, "Failed to connect to uncrypt socket");+                    return false;+                }++                // Read the status from the socket.+                DataInputStream dis = null;+                DataOutputStream dos = null;+                try {+                    dis = new DataInputStream(socket.getInputStream());+                    dos = new DataOutputStream(socket.getOutputStream());+                    int lastStatus = Integer.MIN_VALUE;+                    while (true) {+                        int status = dis.readInt();+                        // Avoid flooding the log with the same message.+                        if (status == lastStatus && lastStatus != Integer.MIN_VALUE) {+                            continue;+                        }+                        lastStatus = status;++                        if (status >= 0 && status <= 100) {+                            // Update status+                            Slog.i(TAG, "uncrypt read status: " + status);+                            if (listener != null) {+                                try {+                                    listener.onProgress(status);+                                } catch (RemoteException ignored) {+                                    Slog.w(TAG, "RemoteException when posting progress");+                                }+                            }+                            if (status == 100) {+                                Slog.i(TAG, "uncrypt successfully finished.");+                                // Ack receipt of the final status code. uncrypt+                                // waits for the ack so the socket won't be+                                // destroyed before we receive the code.+                                dos.writeInt(0);+                                break;+                            }+                        } else {+                            // Error in /system/bin/uncrypt.+                            Slog.e(TAG, "uncrypt failed with status: " + status);+                            // Ack receipt of the final status code. uncrypt waits+                            // for the ack so the socket won't be destroyed before+                            // we receive the code.+                            dos.writeInt(0);+                            return false;+                        }+                    }+                } catch (IOException e) {+                    Slog.e(TAG, "IOException when reading status: ", e);+                    return false;+                } finally {+                    IoUtils.closeQuietly(dis);+                    IoUtils.closeQuietly(dos);+                    IoUtils.closeQuietly(socket);+                }++                return true;+            }         }          @Override // Binder call         public boolean clearBcb() {             if (DEBUG) Slog.d(TAG, "clearBcb");-            return setupOrClearBcb(false, null);+            synchronized (sRequestLock) {+                return setupOrClearBcb(false, null);+            }         }          @Override // Binder call         public boolean setupBcb(String command) {             if (DEBUG) Slog.d(TAG, "setupBcb: [" + command + "]");-            return setupOrClearBcb(true, command);+            synchronized (sRequestLock) {+                return setupOrClearBcb(true, command);+            }+        }++        @Override // Binder call+        public void rebootRecoveryWithCommand(String command) {+            if (DEBUG) Slog.d(TAG, "rebootRecoveryWithCommand: [" + command + "]");+            synchronized (sRequestLock) {+                if (!setupOrClearBcb(true, command)) {+                    return;+                }++                // Having set up the BCB, go ahead and reboot.+                PowerManager pm = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE);+                pm.reboot(PowerManager.REBOOT_RECOVERY);+            }+        }++        /**+         * Check if any of the init services is still running. If so, we cannot+         * start a new uncrypt/setup-bcb/clear-bcb service right away; otherwise+         * it may break the socket communication since init creates / deletes+         * the socket (/dev/socket/uncrypt) on service start / exit.+         */+        private boolean checkAndWaitForUncryptService() {+            for (int retry = 0; retry < SOCKET_CONNECTION_MAX_RETRY; retry++) {+                final String uncryptService = SystemProperties.get(INIT_SERVICE_UNCRYPT);+                final String setupBcbService = SystemProperties.get(INIT_SERVICE_SETUP_BCB);+                final String clearBcbService = SystemProperties.get(INIT_SERVICE_CLEAR_BCB);+                final boolean busy = "running".equals(uncryptService) ||+                        "running".equals(setupBcbService) || "running".equals(clearBcbService);+                if (DEBUG) {+                    Slog.i(TAG, "retry: " + retry + " busy: " + busy ++                            " uncrypt: [" + uncryptService + "]" ++                            " setupBcb: [" + setupBcbService + "]" ++                            " clearBcb: [" + clearBcbService + "]");+                }++                if (!busy) {+                    return true;+                }++                try {+                    Thread.sleep(1000);+                } catch (InterruptedException e) {+                    Slog.w(TAG, "Interrupted:", e);+                }+            }++            return false;         }          private LocalSocket connectService() {@@ -176,7 +244,7 @@                     try {                         Thread.sleep(1000);                     } catch (InterruptedException e) {-                        Slog.w(TAG, "Interrupted: ", e);+                        Slog.w(TAG, "Interrupted:", e);                     }                 }             }@@ -190,6 +258,12 @@         private boolean setupOrClearBcb(boolean isSetup, String command) {             mContext.enforceCallingOrSelfPermission(android.Manifest.permission.RECOVERY, null); +            final boolean available = checkAndWaitForUncryptService();+            if (!available) {+                Slog.e(TAG, "uncrypt service is unavailable.");+                return false;+            }+             if (isSetup) {                 SystemProperties.set("ctl.start", "setup-bcb");             } else {@@ -232,7 +306,7 @@                     return false;                 }             } catch (IOException e) {-                Slog.e(TAG, "IOException when communicating with uncrypt: ", e);+                Slog.e(TAG, "IOException when communicating with uncrypt:", e);                 return false;             } finally {                 IoUtils.closeQuietly(dis);