Sanity-check erroneous backup agent instantiations
Two distinct changes:
Fix a bug seen in the wild where a newly-launched application will be
spuriously asked to instantiate a backup agent. What was happening
there is that some Activity Manager state was being left stale in certain
circumstances, and then in combination with app uninstall / install, there
could be a case where uid reuse wound up looking like an app identity
match.
We now positively verify before instantiating the agent that the intended
backup target package is uid-compatible with the app process that the
instantiation was requested of. The incomplete bookkeeping in the
Activity Manager has also been tightened up, and the Backup Manager is
more aggressive about cleaning up pending operations pertaining to
apps being uninstalled.
Bug 5874010
Change-Id: Ic389f4a96c9dcd0ba6b3962b579084033d8ae9f8
diff --git a/core/java/android/app/ActivityManagerNative.java b/core/java/android/app/ActivityManagerNative.java
index 7492629..67d3930 100644
--- a/core/java/android/app/ActivityManagerNative.java
+++ b/core/java/android/app/ActivityManagerNative.java
@@ -2809,6 +2809,15 @@
return success;
}
+ public void clearPendingBackup() throws RemoteException {
+ Parcel data = Parcel.obtain();
+ Parcel reply = Parcel.obtain();
+ data.writeInterfaceToken(IActivityManager.descriptor);
+ mRemote.transact(CLEAR_PENDING_BACKUP_TRANSACTION, data, reply, 0);
+ reply.recycle();
+ data.recycle();
+ }
+
public void backupAgentCreated(String packageName, IBinder agent) throws RemoteException {
Parcel data = Parcel.obtain();
Parcel reply = Parcel.obtain();
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 5f65f08..456d757 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -29,6 +29,7 @@
import android.content.pm.ApplicationInfo;
import android.content.pm.IPackageManager;
import android.content.pm.InstrumentationInfo;
+import android.content.pm.PackageInfo;
import android.content.pm.PackageManager;
import android.content.pm.PackageManager.NameNotFoundException;
import android.content.pm.ProviderInfo;
@@ -2396,12 +2397,31 @@
private void handleCreateBackupAgent(CreateBackupAgentData data) {
if (DEBUG_BACKUP) Slog.v(TAG, "handleCreateBackupAgent: " + data);
+ // Sanity check the requested target package's uid against ours
+ try {
+ PackageInfo requestedPackage = getPackageManager().getPackageInfo(
+ data.appInfo.packageName, 0, UserHandle.myUserId());
+ if (requestedPackage.applicationInfo.uid != Process.myUid()) {
+ Slog.w(TAG, "Asked to instantiate non-matching package "
+ + data.appInfo.packageName);
+ return;
+ }
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Can't reach package manager", e);
+ return;
+ }
+
// no longer idle; we have backup work to do
unscheduleGcIdler();
// instantiate the BackupAgent class named in the manifest
LoadedApk packageInfo = getPackageInfoNoCheck(data.appInfo, data.compatInfo);
String packageName = packageInfo.mPackageName;
+ if (packageName == null) {
+ Slog.d(TAG, "Asked to create backup agent for nonexistent package");
+ return;
+ }
+
if (mBackupAgents.get(packageName) != null) {
Slog.d(TAG, "BackupAgent " + " for " + packageName
+ " already exists");
diff --git a/core/java/android/app/IActivityManager.java b/core/java/android/app/IActivityManager.java
index 97250e9..8fc1c86 100644
--- a/core/java/android/app/IActivityManager.java
+++ b/core/java/android/app/IActivityManager.java
@@ -152,6 +152,7 @@
public boolean bindBackupAgent(ApplicationInfo appInfo, int backupRestoreMode)
throws RemoteException;
+ public void clearPendingBackup() throws RemoteException;
public void backupAgentCreated(String packageName, IBinder agent) throws RemoteException;
public void unbindBackupAgent(ApplicationInfo appInfo) throws RemoteException;
public void killApplicationProcess(String processName, int uid) throws RemoteException;
@@ -619,4 +620,5 @@
int GET_RUNNING_USER_IDS_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+156;
int REQUEST_BUG_REPORT_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+157;
int INPUT_DISPATCHING_TIMED_OUT_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+158;
+ int CLEAR_PENDING_BACKUP_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+159;
}
diff --git a/services/java/com/android/server/BackupManagerService.java b/services/java/com/android/server/BackupManagerService.java
index 9f01eca..f241c80 100644
--- a/services/java/com/android/server/BackupManagerService.java
+++ b/services/java/com/android/server/BackupManagerService.java
@@ -1474,6 +1474,7 @@
if (MORE_DEBUG) Slog.v(TAG, " removing participant " + packageName);
removeEverBackedUp(packageName);
set.remove(packageName);
+ mPendingBackups.remove(packageName);
}
}
@@ -1625,6 +1626,7 @@
} catch (InterruptedException e) {
// just bail
if (DEBUG) Slog.w(TAG, "Interrupted: " + e);
+ mActivityManager.clearPendingBackup();
return null;
}
}
@@ -1632,6 +1634,7 @@
// if we timed out with no connect, abort and move on
if (mConnecting == true) {
Slog.w(TAG, "Timeout waiting for agent " + app);
+ mActivityManager.clearPendingBackup();
return null;
}
if (DEBUG) Slog.i(TAG, "got agent " + mConnectedAgent);
diff --git a/services/java/com/android/server/am/ActivityManagerService.java b/services/java/com/android/server/am/ActivityManagerService.java
index 7132e1e..1737876a 100644
--- a/services/java/com/android/server/am/ActivityManagerService.java
+++ b/services/java/com/android/server/am/ActivityManagerService.java
@@ -11119,8 +11119,8 @@
// instantiated. The backup agent will invoke backupAgentCreated() on the
// activity manager to announce its creation.
public boolean bindBackupAgent(ApplicationInfo app, int backupMode) {
- if (DEBUG_BACKUP) Slog.v(TAG, "startBackupAgent: app=" + app + " mode=" + backupMode);
- enforceCallingPermission("android.permission.BACKUP", "startBackupAgent");
+ if (DEBUG_BACKUP) Slog.v(TAG, "bindBackupAgent: app=" + app + " mode=" + backupMode);
+ enforceCallingPermission("android.permission.BACKUP", "bindBackupAgent");
synchronized(this) {
// !!! TODO: currently no check here that we're already bound
@@ -11181,6 +11181,17 @@
return true;
}
+ @Override
+ public void clearPendingBackup() {
+ if (DEBUG_BACKUP) Slog.v(TAG, "clearPendingBackup");
+ enforceCallingPermission("android.permission.BACKUP", "clearPendingBackup");
+
+ synchronized (this) {
+ mBackupTarget = null;
+ mBackupAppName = null;
+ }
+ }
+
// A backup agent has just come up
public void backupAgentCreated(String agentPackageName, IBinder agent) {
if (DEBUG_BACKUP) Slog.v(TAG, "backupAgentCreated: " + agentPackageName
@@ -11217,32 +11228,34 @@
}
synchronized(this) {
- if (mBackupAppName == null) {
- Slog.w(TAG, "Unbinding backup agent with no active backup");
- return;
- }
-
- if (!mBackupAppName.equals(appInfo.packageName)) {
- Slog.e(TAG, "Unbind of " + appInfo + " but is not the current backup target");
- return;
- }
-
- ProcessRecord proc = mBackupTarget.app;
- mBackupTarget = null;
- mBackupAppName = null;
-
- // Not backing this app up any more; reset its OOM adjustment
- updateOomAdjLocked(proc);
-
- // If the app crashed during backup, 'thread' will be null here
- if (proc.thread != null) {
- try {
- proc.thread.scheduleDestroyBackupAgent(appInfo,
- compatibilityInfoForPackageLocked(appInfo));
- } catch (Exception e) {
- Slog.e(TAG, "Exception when unbinding backup agent:");
- e.printStackTrace();
+ try {
+ if (mBackupAppName == null) {
+ Slog.w(TAG, "Unbinding backup agent with no active backup");
+ return;
}
+
+ if (!mBackupAppName.equals(appInfo.packageName)) {
+ Slog.e(TAG, "Unbind of " + appInfo + " but is not the current backup target");
+ return;
+ }
+
+ // Not backing this app up any more; reset its OOM adjustment
+ final ProcessRecord proc = mBackupTarget.app;
+ updateOomAdjLocked(proc);
+
+ // If the app crashed during backup, 'thread' will be null here
+ if (proc.thread != null) {
+ try {
+ proc.thread.scheduleDestroyBackupAgent(appInfo,
+ compatibilityInfoForPackageLocked(appInfo));
+ } catch (Exception e) {
+ Slog.e(TAG, "Exception when unbinding backup agent:");
+ e.printStackTrace();
+ }
+ }
+ } finally {
+ mBackupTarget = null;
+ mBackupAppName = null;
}
}
}