From 4831ad7039b068a3dc91c5bb9bd92a0f46800e77 Mon Sep 17 00:00:00 2001 From: shafik Date: Fri, 3 May 2019 17:36:42 +0100 Subject: [PATCH] Fail to enable rollback if enable rollback times out Make PackageManager send a ACTION_CANCEL_ENABLE_ROLLBACK intent to RollbackManager. RollbackManager marks the relevant rollback as invalid. Allow enable rollback to continue as usual, before making the rollback available, RollbackManager checks whether it's valid. If it's not, the rollback data is deleted. Add a test case for expired rollback enabling attempt in RollbackTest. Test: atest RollbackTest#testEnableRollbackTimeoutFailsRollback Test: manual - * Set ENABLE_ROLLBACK_TIMEOUT_MILLIS to 1 ms using DeviceConfig * Install a mainline module with rollback enabled * adb shell dumpsys rollback * observe that no rollback was made available Fixes: 131679409 Change-Id: Iaa4dbff002b820aff1fc3e1b985f129cf5ebe2e6 --- core/java/android/content/Intent.java | 12 ++++ .../content/rollback/RollbackManager.java | 4 +- core/res/AndroidManifest.xml | 1 + .../server/pm/PackageManagerService.java | 9 +++ .../rollback/RollbackManagerServiceImpl.java | 62 ++++++++++++++++++- .../android/tests/rollback/RollbackTest.java | 39 ++++++++++++ 6 files changed, 122 insertions(+), 5 deletions(-) diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index 389832815d777..50d1785c60596 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -2444,6 +2444,18 @@ public class Intent implements Parcelable, Cloneable { @SdkConstant(SdkConstantType.BROADCAST_INTENT_ACTION) public static final String ACTION_PACKAGE_ENABLE_ROLLBACK = "android.intent.action.PACKAGE_ENABLE_ROLLBACK"; + /** + * Broadcast Action: Sent to the system rollback manager when the rollback for a certain + * package needs to be cancelled. + * + *

This intent is sent by PackageManagerService to notify RollbackManager + * that enabling a specific rollback has timed out. + * + * @hide + */ + @SdkConstant(SdkConstantType.BROADCAST_INTENT_ACTION) + public static final String ACTION_CANCEL_ENABLE_ROLLBACK = + "android.intent.action.CANCEL_ENABLE_ROLLBACK"; /** * Broadcast Action: A rollback has been committed. * diff --git a/core/java/android/content/rollback/RollbackManager.java b/core/java/android/content/rollback/RollbackManager.java index d54a6fe0a7b2f..9a10a0c4766b8 100644 --- a/core/java/android/content/rollback/RollbackManager.java +++ b/core/java/android/content/rollback/RollbackManager.java @@ -40,7 +40,7 @@ import java.util.List; * used to initiate rollback of those packages for a limited time period after * upgrade. * - * @see PackageInstaller.SessionParams#setEnableRollback() + * @see PackageInstaller.SessionParams#setEnableRollback(boolean) * @hide */ @SystemApi @TestApi @@ -52,7 +52,7 @@ public final class RollbackManager { /** * Lifetime duration of rollback packages in millis. A rollback will be available for * at most that duration of time after a package is installed with - * {@link PackageInstaller.SessionParams#setEnableRollback()}. + * {@link PackageInstaller.SessionParams#setEnableRollback(boolean)}. * *

If flag value is negative, the default value will be assigned. * diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index 8714bf2505bb5..57b7704558171 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -43,6 +43,7 @@ + diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index a64ae9c5fe858..baf4fbb470643 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -1744,6 +1744,15 @@ public class PackageManagerService extends IPackageManager.Stub Trace.asyncTraceEnd( TRACE_TAG_PACKAGE_MANAGER, "enable_rollback", enableRollbackToken); params.handleRollbackEnabled(); + Intent rollbackTimeoutIntent = new Intent( + Intent.ACTION_CANCEL_ENABLE_ROLLBACK); + rollbackTimeoutIntent.putExtra( + PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_TOKEN, + enableRollbackToken); + rollbackTimeoutIntent.addFlags( + Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); + mContext.sendBroadcastAsUser(rollbackTimeoutIntent, UserHandle.SYSTEM, + android.Manifest.permission.PACKAGE_ROLLBACK_AGENT); } break; } diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index d4c7ec355c197..99a0640d45b1a 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -184,7 +184,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { getHandler().post(() -> { boolean success = enableRollback(installFlags, newPackageCodePath, - installedUsers, user); + installedUsers, user, token); int ret = PackageManagerInternal.ENABLE_ROLLBACK_SUCCEEDED; if (!success) { ret = PackageManagerInternal.ENABLE_ROLLBACK_FAILED; @@ -203,6 +203,27 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } }, enableRollbackFilter, null, getHandler()); + IntentFilter enableRollbackTimedOutFilter = new IntentFilter(); + enableRollbackTimedOutFilter.addAction(Intent.ACTION_CANCEL_ENABLE_ROLLBACK); + + mContext.registerReceiver(new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + if (Intent.ACTION_CANCEL_ENABLE_ROLLBACK.equals(intent.getAction())) { + int token = intent.getIntExtra( + PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_TOKEN, -1); + synchronized (mLock) { + for (NewRollback rollback : mNewRollbacks) { + if (rollback.hasToken(token)) { + rollback.isCancelled = true; + return; + } + } + } + } + } + }, enableRollbackTimedOutFilter, null, getHandler()); + registerTimeChangeReceiver(); } @@ -807,10 +828,11 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { * @param newPackageCodePath path to the package about to be installed. * @param installedUsers the set of users for which a given package is installed. * @param user the user that owns the install session to enable rollback on. + * @param token the distinct rollback token sent by package manager. * @return true if enabling the rollback succeeds, false otherwise. */ private boolean enableRollback(int installFlags, File newPackageCodePath, - int[] installedUsers, @UserIdInt int user) { + int[] installedUsers, @UserIdInt int user, int token) { // Find the session id associated with this install. // TODO: It would be nice if package manager or package installer told @@ -903,6 +925,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { mNewRollbacks.add(newRollback); } } + newRollback.addToken(token); return enableRollbackForPackageSession(newRollback.data, packageSession, installedUsers, /* snapshotUserData*/ true); @@ -1005,7 +1028,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { public void restoreUserData(String packageName, int[] userIds, int appId, long ceDataInode, String seInfo, int token) { if (Binder.getCallingUid() != Process.SYSTEM_UID) { - throw new SecurityException("restoureUserData may only be called by the system."); + throw new SecurityException("restoreUserData may only be called by the system."); } getHandler().post(() -> { @@ -1261,6 +1284,11 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { deleteRollback(data); return null; } + if (newRollback.isCancelled) { + Log.e(TAG, "Rollback has been cancelled by PackageManager"); + deleteRollback(data); + return null; + } // It's safe to access data.info outside a synchronized block because // this is running on the handler thread and all changes to the @@ -1440,6 +1468,13 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { private static class NewRollback { public final RollbackData data; + /** + * This array holds all of the rollback tokens associated with package sessions included + * in this rollback. This is used to identify which rollback should be cancelled in case + * {@link PackageManager} sends an {@link Intent#ACTION_CANCEL_ENABLE_ROLLBACK} intent. + */ + private final IntArray mTokens = new IntArray(); + /** * Session ids for all packages in the install. * For multi-package sessions, this is the list of child session ids. @@ -1448,10 +1483,31 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { */ public final int[] packageSessionIds; + /** + * Flag to determine whether the RollbackData has been cancelled. + * + *

RollbackData could be invalidated and cancelled if RollbackManager receives + * {@link Intent#ACTION_CANCEL_ENABLE_ROLLBACK} from {@link PackageManager}. + * + *

The main underlying assumption here is that if enabling the rollback times out, then + * {@link PackageManager} will NOT send + * {@link PackageInstaller.SessionCallback#onFinished(int, boolean)} before it broadcasts + * {@link Intent#ACTION_CANCEL_ENABLE_ROLLBACK}. + */ + public boolean isCancelled = false; + NewRollback(RollbackData data, int[] packageSessionIds) { this.data = data; this.packageSessionIds = packageSessionIds; } + + public void addToken(int token) { + mTokens.add(token); + } + + public boolean hasToken(int token) { + return mTokens.indexOf(token) != -1; + } } NewRollback createNewRollbackLocked(PackageInstaller.SessionInfo parentSession) { diff --git a/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java b/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java index 13b5b9ac104e9..beff0c6bc3088 100644 --- a/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java +++ b/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java @@ -35,6 +35,7 @@ import android.content.pm.VersionedPackage; import android.content.rollback.RollbackInfo; import android.content.rollback.RollbackManager; import android.provider.DeviceConfig; +import android.provider.Settings; import android.util.Log; import androidx.test.InstrumentationRegistry; @@ -60,6 +61,12 @@ public class RollbackTest { private static final String TEST_APP_B = "com.android.tests.rollback.testapp.B"; private static final String INSTRUMENTED_APP = "com.android.tests.rollback"; + // copied from PackageManagerService#PROPERTY_ENABLE_ROLLBACK_TIMEOUT_MILLIS + // TODO: find a better place for the property so that it can be imported in tests + // maybe android.content.pm.PackageManager? + private static final String PROPERTY_ENABLE_ROLLBACK_TIMEOUT_MILLIS = + "enable_rollback_timeout"; + /** * Test basic rollbacks. */ @@ -952,6 +959,38 @@ public class RollbackTest { } } + @Test + public void testEnableRollbackTimeoutFailsRollback() throws Exception { + try { + RollbackTestUtils.adoptShellPermissionIdentity( + Manifest.permission.INSTALL_PACKAGES, + Manifest.permission.DELETE_PACKAGES, + Manifest.permission.TEST_MANAGE_ROLLBACKS, + Manifest.permission.MANAGE_ROLLBACKS, + Manifest.permission.WRITE_DEVICE_CONFIG); + + //setting the timeout to a very short amount that will definitely be triggered + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ROLLBACK, + PROPERTY_ENABLE_ROLLBACK_TIMEOUT_MILLIS, + Long.toString(1), false /* makeDefault*/); + RollbackManager rm = RollbackTestUtils.getRollbackManager(); + + RollbackTestUtils.uninstall(TEST_APP_A); + RollbackTestUtils.install("RollbackTestAppAv1.apk", false); + RollbackTestUtils.install("RollbackTestAppAv2.apk", true); + + assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); + + assertNull(getUniqueRollbackInfoForPackage(rm.getAvailableRollbacks(), TEST_APP_A)); + } finally { + //setting the timeout back to default + DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ROLLBACK, + PROPERTY_ENABLE_ROLLBACK_TIMEOUT_MILLIS, + null, false /* makeDefault*/); + RollbackTestUtils.dropShellPermissionIdentity(); + } + } + // Helper function to test that the given rollback info is a rollback for // the atomic set {A2, B2} -> {A1, B1}. private void assertRollbackInfoForAandB(RollbackInfo rollback) {