From 9f7a0acd2d383b825bcdcbef25d6b42f9875acfe Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 13 Jan 2017 09:45:14 -0800 Subject: [PATCH] RecoverySystem: Fix the issue in installPackage(). Commit 794c8b0b3fe16051843c22232d58d6b184dde49b fixed the race condition when requesting data wipes via uncrypt. We have similar issue with RecoverySystem.installPackage(). It first requests to set up the BCB, then triggers a reboot. These two steps should finish atomically. This CL switches to calling RecoverySystemService.rebootRecoveryWithCommand(), which guards the two steps with synchronized blocks. Bug: 34239871 Test: Having two apps: one calls RecoverySystem.cancelScheduledUpdate() continuously, and the other calls RecoverySystem.installPackage() just once. The install request should not be cancelled by the other. Change-Id: I5ec56fcaa70eae7c33e3cc8e6cfc7472b935ce4e --- core/java/android/os/IRecoverySystem.aidl | 2 +- core/java/android/os/RecoverySystem.java | 15 +++++---------- .../com/android/server/RecoverySystemService.java | 7 +++++-- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/core/java/android/os/IRecoverySystem.aidl b/core/java/android/os/IRecoverySystem.aidl index c5ceecd7c8b95..1ee83ae2847d6 100644 --- a/core/java/android/os/IRecoverySystem.aidl +++ b/core/java/android/os/IRecoverySystem.aidl @@ -25,5 +25,5 @@ interface IRecoverySystem { boolean uncrypt(in String packageFile, IRecoverySystemProgressListener listener); boolean setupBcb(in String command); boolean clearBcb(); - void rebootRecoveryWithCommand(in String command); + void rebootRecoveryWithCommand(in String command, in boolean update); } diff --git a/core/java/android/os/RecoverySystem.java b/core/java/android/os/RecoverySystem.java index d48431afe6913..7f9ea438cb95e 100644 --- a/core/java/android/os/RecoverySystem.java +++ b/core/java/android/os/RecoverySystem.java @@ -491,15 +491,10 @@ public class RecoverySystem { command += securityArg; } + // RECOVERY_SERVICE writes to BCB (bootloader control block) and triggers the reboot. RecoverySystem rs = (RecoverySystem) context.getSystemService( Context.RECOVERY_SERVICE); - if (!rs.setupBcb(command)) { - throw new IOException("Setup BCB failed"); - } - - // Having set up the BCB (bootloader control block), go ahead and reboot - PowerManager pm = (PowerManager) context.getSystemService(Context.POWER_SERVICE); - pm.reboot(PowerManager.REBOOT_RECOVERY_UPDATE); + rs.rebootRecoveryWithCommand(command, true /* update */); throw new IOException("Reboot failed (no permissions?)"); } @@ -713,7 +708,7 @@ public class RecoverySystem { // 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()); + rs.rebootRecoveryWithCommand(command.toString(), false); throw new IOException("Reboot failed (no permissions?)"); } @@ -913,9 +908,9 @@ public class RecoverySystem { * Talks to RecoverySystemService via Binder to set up the BCB command and * reboot into recovery accordingly. */ - private void rebootRecoveryWithCommand(String command) { + private void rebootRecoveryWithCommand(String command, boolean update) { try { - mService.rebootRecoveryWithCommand(command); + mService.rebootRecoveryWithCommand(command, update); } catch (RemoteException ignored) { } } diff --git a/services/core/java/com/android/server/RecoverySystemService.java b/services/core/java/com/android/server/RecoverySystemService.java index 3c8c699a65bb7..2010e647e1eab 100644 --- a/services/core/java/com/android/server/RecoverySystemService.java +++ b/services/core/java/com/android/server/RecoverySystemService.java @@ -181,7 +181,7 @@ public final class RecoverySystemService extends SystemService { } @Override // Binder call - public void rebootRecoveryWithCommand(String command) { + public void rebootRecoveryWithCommand(String command, boolean update) { if (DEBUG) Slog.d(TAG, "rebootRecoveryWithCommand: [" + command + "]"); synchronized (sRequestLock) { if (!setupOrClearBcb(true, command)) { @@ -190,7 +190,10 @@ public final class RecoverySystemService extends SystemService { // Having set up the BCB, go ahead and reboot. PowerManager pm = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE); - pm.reboot(PowerManager.REBOOT_RECOVERY); + // PowerManagerService may additionally request uncrypting the package when it's + // to install an update (REBOOT_RECOVERY_UPDATE). + pm.reboot(update ? PowerManager.REBOOT_RECOVERY_UPDATE : + PowerManager.REBOOT_RECOVERY); } }