Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 64d73c4f authored by Sarp Misoglu's avatar Sarp Misoglu
Browse files

Ensure we bind to the correct app's agent

We have a 10s timeout in bindToAgentSynchronous and if we don't get an
agentConnected callback by then the method returns null. There was a
potential bug here because agentConnected doesn't check if the agent is
the one we are currently expecting. It could happen with the following
sequence of events:

1. bindToAgentSynchronous(A)
2. app A times out
3. bindToAgentSynchronous(B)
4. agentConnected(A)
5. the call from 3 returns the agent for app A instead of B.

With this change we start tracking the state of the current connection
better.

Test: atest BackupAgentConnectionManagerTest & atest
CtsBackupHostTestCases
Bug: 328019933
Change-Id: I0556aff00fe8c2bbfe56fe10f15b7b15ff260590

Change-Id: I4c04e9279d9863716fc080c6ab218665e1591d51
parent 1ba241be
Loading
Loading
Loading
Loading
+97 −51
Original line number Diff line number Diff line
@@ -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);
                startedBindSuccessfully = mActivityManager.bindBackupAgent(app.packageName, mode,
                        mUserId, backupDestination, useRestrictedMode);
            } catch (RemoteException e) {
                // can't happen - ActivityManager is local
            }

                    // success; wait for the agent to arrive
                    // only wait 10 seconds for the bind to happen
            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 (mConnecting && mConnectedAgent == null && (System.currentTimeMillis()
                            < timeoutMark)) {
                while (mCurrentConnection != null && mCurrentConnection.connecting && (
                        System.currentTimeMillis() < timeoutMark)) {
                    try {
                        mAgentConnectLock.wait(5000);
                    } catch (InterruptedException e) {
                            // just bail
                        Slog.w(TAG, mUserIdMsg + "Interrupted: " + e);
                            mConnecting = false;
                            mConnectedAgent = null;
                        mCurrentConnection = null;
                    }
                }

                    // if we timed out with no connect, abort and move on
                    if (mConnecting) {
                        Slog.w(TAG, mUserIdMsg + "Timeout waiting for agent " + app);
                        mConnectedAgent = null;
                    }
                    Slog.i(TAG, mUserIdMsg + "got agent " + mConnectedAgent);
                    agent = mConnectedAgent;
            }
            } catch (RemoteException e) {
                // can't happen - ActivityManager is local

            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;
            }
        if (agent == 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) {
        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.
+26 −0
Original line number Diff line number Diff line
@@ -195,6 +195,31 @@ public class BackupAgentConnectionManagerTest {
        verify(mActivityManagerInternal, never()).clearPendingBackup(anyInt());
    }

    @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()
@@ -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 -> {