From a4ce433eae59ce59850e378125c28882059e458b Mon Sep 17 00:00:00 2001
From: JW Wang <wangchun@google.com>
Date: Tue, 29 Oct 2019 21:26:03 +0800
Subject: [PATCH] Remove rollbacks with invalid sessions

This could happen to sessions with rollback enabled that are abandoned
before reboot. We need to remove them from mRollbacks so we won't do
anything funny when user is unlocked to cause crash.

See b/143516169 #1 for a detailed explanation for how crash happens.

Bug: 143516169
Test: atest StagedRollbackTest#testPreviouslyAbandonedRollbacks
and search device logs for "FATAL EXCEPTION IN SYSTEM PROCESS" to make
sure system_server doesn't crash inside RollbackManager.

Change-Id: I2c8371be6a9d0e90459ee8f4695f14a94b3aeaca
---
 .../rollback/RollbackManagerServiceImpl.java  | 45 +++++++++----------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
index ef4c12e8145a8..1fd15d9e765fc 100644
--- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
+++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
@@ -469,44 +469,39 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
             List<Rollback> restoreInProgress = new ArrayList<>();
             Set<String> apexPackageNames = new HashSet<>();
             synchronized (mLock) {
-                for (Rollback rollback : mRollbacks) {
-                    if (rollback.isStaged()) {
+                Iterator<Rollback> iter = mRollbacks.iterator();
+                while (iter.hasNext()) {
+                    Rollback rollback = iter.next();
+                    if (!rollback.isStaged()) {
+                        // We only care about staged rollbacks here
+                        continue;
+                    }
+
+                    PackageInstaller.SessionInfo session = mContext.getPackageManager()
+                            .getPackageInstaller().getSessionInfo(rollback.getStagedSessionId());
+                    if (session == null || session.isStagedSessionFailed()) {
+                        iter.remove();
+                        rollback.delete(mAppDataRollbackHelper);
+                        continue;
+                    }
+
+                    if (session.isStagedSessionApplied()) {
                         if (rollback.isEnabling()) {
                             enabling.add(rollback);
                         } else if (rollback.isRestoreUserDataInProgress()) {
                             restoreInProgress.add(rollback);
                         }
-
-                        apexPackageNames.addAll(rollback.getApexPackageNames());
                     }
+                    apexPackageNames.addAll(rollback.getApexPackageNames());
                 }
             }
 
             for (Rollback rollback : enabling) {
-                PackageInstaller installer = mContext.getPackageManager().getPackageInstaller();
-                PackageInstaller.SessionInfo session =
-                        installer.getSessionInfo(rollback.getStagedSessionId());
-                if (session == null || session.isStagedSessionFailed()) {
-                    // TODO: Do we need to remove this from
-                    // mRollbacks, or is it okay to leave as
-                    // unavailable until the next reboot when it will go
-                    // away on its own?
-                    rollback.delete(mAppDataRollbackHelper);
-                } else if (session.isStagedSessionApplied()) {
-                    makeRollbackAvailable(rollback);
-                }
+                makeRollbackAvailable(rollback);
             }
 
             for (Rollback rollback : restoreInProgress) {
-                PackageInstaller installer = mContext.getPackageManager().getPackageInstaller();
-                PackageInstaller.SessionInfo session =
-                        installer.getSessionInfo(rollback.getStagedSessionId());
-                // TODO: What if session is null?
-                if (session != null) {
-                    if (session.isStagedSessionApplied() || session.isStagedSessionFailed()) {
-                        rollback.setRestoreUserDataInProgress(false);
-                    }
-                }
+                rollback.setRestoreUserDataInProgress(false);
             }
 
             for (String apexPackageName : apexPackageNames) {