diff options
2 files changed, 123 insertions, 51 deletions
diff --git a/services/backup/java/com/android/server/backup/BackupAgentConnectionManager.java b/services/backup/java/com/android/server/backup/BackupAgentConnectionManager.java index 5e4bab15952d..02f186557ca4 100644 --- a/services/backup/java/com/android/server/backup/BackupAgentConnectionManager.java +++ b/services/backup/java/com/android/server/backup/BackupAgentConnectionManager.java @@ -41,6 +41,7 @@ import android.os.UserHandle; import android.util.ArraySet; import android.util.Slog; +import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.server.LocalServices; import com.android.server.backup.internal.LifecycleOperationStorage; @@ -71,8 +72,10 @@ public class BackupAgentConnectionManager { // Activity Manager; use this lock object to signal when a requested binding has // completed. private final Object mAgentConnectLock = new Object(); - private IBackupAgent mConnectedAgent; - private volatile boolean mConnecting; + @GuardedBy("mAgentConnectLock") + @Nullable + private BackupAgentConnection mCurrentConnection; + private final ArraySet<String> mRestoreNoRestrictedModePackages = new ArraySet<>(); private final ArraySet<String> mBackupNoRestrictedModePackages = new ArraySet<>(); @@ -96,8 +99,18 @@ public class BackupAgentConnectionManager { mUserIdMsg = "[UserID:" + userId + "] "; } + private static final class BackupAgentConnection { + public final ApplicationInfo appInfo; + public IBackupAgent backupAgent; + public boolean connecting = true; // Assume we are trying to connect on creation. + + private BackupAgentConnection(ApplicationInfo appInfo) { + this.appInfo = appInfo; + } + } + /** - * Fires off a backup agent, blocking until it attaches (and ActivityManager will call + * Fires off a backup agent, blocking until it attaches (i.e. ActivityManager calls * {@link #agentConnected(String, IBinder)}) or until this operation times out. * * @param mode a {@code BACKUP_MODE} from {@link android.app.ApplicationThreadConstants}. @@ -105,48 +118,56 @@ public class BackupAgentConnectionManager { @Nullable public IBackupAgent bindToAgentSynchronous(ApplicationInfo app, int mode, @BackupAnnotations.BackupDestination int backupDestination) { - IBackupAgent agent = null; synchronized (mAgentConnectLock) { - mConnecting = true; - mConnectedAgent = null; boolean useRestrictedMode = shouldUseRestrictedBackupModeForPackage(mode, app.packageName); + if (mCurrentConnection != null) { + Slog.e(TAG, mUserIdMsg + "binding to new agent before unbinding from old one: " + + mCurrentConnection.appInfo.packageName); + } + mCurrentConnection = new BackupAgentConnection(app); + + // bindBackupAgent() is an async API. It will kick off the app's process and call + // agentConnected() when it receives the agent from the app. + boolean startedBindSuccessfully = false; try { - if (mActivityManager.bindBackupAgent(app.packageName, mode, mUserId, - backupDestination, useRestrictedMode)) { - Slog.d(TAG, mUserIdMsg + "awaiting agent for " + app); - - // success; wait for the agent to arrive - // only wait 10 seconds for the bind to happen - long timeoutMark = System.currentTimeMillis() + 10 * 1000; - while (mConnecting && mConnectedAgent == null && (System.currentTimeMillis() - < timeoutMark)) { - try { - mAgentConnectLock.wait(5000); - } catch (InterruptedException e) { - // just bail - Slog.w(TAG, mUserIdMsg + "Interrupted: " + e); - mConnecting = false; - mConnectedAgent = null; - } - } + startedBindSuccessfully = mActivityManager.bindBackupAgent(app.packageName, mode, + mUserId, backupDestination, useRestrictedMode); + } catch (RemoteException e) { + // can't happen - ActivityManager is local + } - // if we timed out with no connect, abort and move on - if (mConnecting) { - Slog.w(TAG, mUserIdMsg + "Timeout waiting for agent " + app); - mConnectedAgent = null; + if (!startedBindSuccessfully) { + Slog.w(TAG, mUserIdMsg + "bind request failed for " + app.packageName); + mCurrentConnection = null; + } else { + Slog.d(TAG, mUserIdMsg + "awaiting agent for " + app.packageName); + + // Wait 10 seconds for the agent and then time out if we still haven't bound to it. + long timeoutMark = System.currentTimeMillis() + 10 * 1000; + while (mCurrentConnection != null && mCurrentConnection.connecting && ( + System.currentTimeMillis() < timeoutMark)) { + try { + mAgentConnectLock.wait(5000); + } catch (InterruptedException e) { + Slog.w(TAG, mUserIdMsg + "Interrupted: " + e); + mCurrentConnection = null; } - Slog.i(TAG, mUserIdMsg + "got agent " + mConnectedAgent); - agent = mConnectedAgent; } - } catch (RemoteException e) { - // can't happen - ActivityManager is local } - } - if (agent == null) { + + if (mCurrentConnection != null) { + if (!mCurrentConnection.connecting) { + return mCurrentConnection.backupAgent; + } + // If we are still connecting, we've timed out. + Slog.w(TAG, mUserIdMsg + "Timeout waiting for agent " + app); + mCurrentConnection = null; + } + mActivityManagerInternal.clearPendingBackup(mUserId); + return null; } - return agent; } /** @@ -154,30 +175,49 @@ public class BackupAgentConnectionManager { * It will tell the app to destroy the agent. */ public void unbindAgent(ApplicationInfo app) { - try { - mActivityManager.unbindBackupAgent(app); - } catch (RemoteException e) { - // Can't happen - activity manager is local + synchronized (mAgentConnectLock) { + if (mCurrentConnection == null) { + Slog.w(TAG, mUserIdMsg + "unbindAgent but no current connection"); + } else if (!mCurrentConnection.appInfo.packageName.equals(app.packageName)) { + Slog.w(TAG, mUserIdMsg + "unbindAgent for unexpected package: " + app.packageName + + " expected: " + mCurrentConnection.appInfo.packageName); + } else { + mCurrentConnection = null; + } + + // Even if we weren't expecting to be bound to this agent, we should still call + // ActivityManager just in case. It will ignore the call if it also wasn't expecting it. + try { + mActivityManager.unbindBackupAgent(app); + } catch (RemoteException e) { + // Can't happen - activity manager is local + } } } /** * Callback: a requested backup agent has been instantiated. This should only be called from - * the - * {@link ActivityManager} when it's telling us that an agent is ready after a call to + * the {@link ActivityManager} when it's telling us that an agent is ready after a call to * {@link #bindToAgentSynchronous(ApplicationInfo, int, int)}. */ public void agentConnected(String packageName, IBinder agentBinder) { synchronized (mAgentConnectLock) { - if (getCallingUid() == android.os.Process.SYSTEM_UID) { - Slog.d(TAG, - mUserIdMsg + "agentConnected pkg=" + packageName + " agent=" + agentBinder); - mConnectedAgent = IBackupAgent.Stub.asInterface(agentBinder); - mConnecting = false; - } else { + if (getCallingUid() != Process.SYSTEM_UID) { Slog.w(TAG, mUserIdMsg + "Non-system process uid=" + getCallingUid() + " claiming agent connected"); + return; } + + Slog.d(TAG, mUserIdMsg + "agentConnected pkg=" + packageName + " agent=" + agentBinder); + if (mCurrentConnection == null) { + Slog.w(TAG, mUserIdMsg + "was not expecting connection"); + } else if (!mCurrentConnection.appInfo.packageName.equals(packageName)) { + Slog.w(TAG, mUserIdMsg + "got agent for unexpected package=" + packageName); + } else { + mCurrentConnection.backupAgent = IBackupAgent.Stub.asInterface(agentBinder); + mCurrentConnection.connecting = false; + } + mAgentConnectLock.notifyAll(); } } @@ -189,16 +229,22 @@ public class BackupAgentConnectionManager { */ public void agentDisconnected(String packageName) { synchronized (mAgentConnectLock) { - if (getCallingUid() == Process.SYSTEM_UID) { - mConnectedAgent = null; - mConnecting = false; - } else { + if (getCallingUid() != Process.SYSTEM_UID) { Slog.w(TAG, mUserIdMsg + "Non-system process uid=" + getCallingUid() + " claiming agent disconnected"); + return; } + Slog.w(TAG, mUserIdMsg + "agentDisconnected: the backup agent for " + packageName + " died: cancel current operations"); + // Only abort the current connection if the agent we were expecting or already + // connected to has disconnected. + if (mCurrentConnection != null && mCurrentConnection.appInfo.packageName.equals( + packageName)) { + mCurrentConnection = null; + } + // Offload operation cancellation off the main thread as the cancellation callbacks // might call out to BackupTransport. Other operations started on the same package // before the cancellation callback has executed will also be cancelled by the callback. diff --git a/services/tests/mockingservicestests/src/com/android/server/backup/BackupAgentConnectionManagerTest.java b/services/tests/mockingservicestests/src/com/android/server/backup/BackupAgentConnectionManagerTest.java index 19e43b6fa851..2461e9e79acf 100644 --- a/services/tests/mockingservicestests/src/com/android/server/backup/BackupAgentConnectionManagerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/backup/BackupAgentConnectionManagerTest.java @@ -196,6 +196,31 @@ public class BackupAgentConnectionManagerTest { } @Test + public void bindToAgentSynchronous_unexpectedAgentConnected_doesNotReturnWrongAgent() + throws Exception { + when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), + anyInt(), anyBoolean())).thenReturn(true); + // This is so that IBackupAgent.Stub.asInterface() works. + when(mBackupAgentStub.queryLocalInterface(any())).thenReturn(mBackupAgentStub); + when(mConnectionManager.getCallingUid()).thenReturn(Process.SYSTEM_UID); + + // This is going to block until it receives the callback so we need to run it on a + // separate thread. + Thread testThread = new Thread(() -> setBackupAgentResult( + mConnectionManager.bindToAgentSynchronous(mTestApplicationInfo, + ApplicationThreadConstants.BACKUP_MODE_FULL, BackupDestination.CLOUD)), + "backup-agent-connection-manager-test"); + testThread.start(); + // Give the testThread a head start, otherwise agentConnected() might run before + // bindToAgentSynchronous() is called. + Thread.sleep(500); + mConnectionManager.agentConnected("com.other.package", mBackupAgentStub); + testThread.join(100); // Avoid waiting the full timeout. + + assertThat(mBackupAgentResult).isNull(); + } + + @Test @DisableFlags(Flags.FLAG_ENABLE_RESTRICTED_MODE_CHANGES) public void bindToAgentSynchronous_restrictedModeChangesFlagOff_shouldUseRestrictedMode() throws Exception { @@ -345,6 +370,7 @@ public class BackupAgentConnectionManagerTest { @Test public void agentDisconnected_cancelsCurrentOperations() throws Exception { + when(mConnectionManager.getCallingUid()).thenReturn(Process.SYSTEM_UID); when(mOperationStorage.operationTokensForPackage(eq(TEST_PACKAGE))).thenReturn( ImmutableSet.of(123, 456, 789)); when(mConnectionManager.getThreadForCancellation(any())).thenAnswer(invocation -> { |