Skip to content

Commit 346acb1

Browse files
author
Christopher Tate
committed
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
1 parent 69b0c97 commit 346acb1

File tree

5 files changed

+71
-24
lines changed

5 files changed

+71
-24
lines changed

‎core/java/android/app/ActivityManagerNative.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2809,6 +2809,15 @@ public boolean bindBackupAgent(ApplicationInfo app, int backupRestoreMode)
28092809
return success;
28102810
}
28112811

2812+
public void clearPendingBackup() throws RemoteException {
2813+
Parcel data = Parcel.obtain();
2814+
Parcel reply = Parcel.obtain();
2815+
data.writeInterfaceToken(IActivityManager.descriptor);
2816+
mRemote.transact(CLEAR_PENDING_BACKUP_TRANSACTION, data, reply, 0);
2817+
reply.recycle();
2818+
data.recycle();
2819+
}
2820+
28122821
public void backupAgentCreated(String packageName, IBinder agent) throws RemoteException {
28132822
Parcel data = Parcel.obtain();
28142823
Parcel reply = Parcel.obtain();

‎core/java/android/app/ActivityThread.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import android.content.pm.ApplicationInfo;
3030
import android.content.pm.IPackageManager;
3131
import android.content.pm.InstrumentationInfo;
32+
import android.content.pm.PackageInfo;
3233
import android.content.pm.PackageManager;
3334
import android.content.pm.PackageManager.NameNotFoundException;
3435
import android.content.pm.ProviderInfo;
@@ -2396,12 +2397,31 @@ private void handleReceiver(ReceiverData data) {
23962397
private void handleCreateBackupAgent(CreateBackupAgentData data) {
23972398
if (DEBUG_BACKUP) Slog.v(TAG, "handleCreateBackupAgent: " + data);
23982399

2400+
// Sanity check the requested target package's uid against ours
2401+
try {
2402+
PackageInfo requestedPackage = getPackageManager().getPackageInfo(
2403+
data.appInfo.packageName, 0, UserHandle.myUserId());
2404+
if (requestedPackage.applicationInfo.uid != Process.myUid()) {
2405+
Slog.w(TAG, "Asked to instantiate non-matching package "
2406+
+ data.appInfo.packageName);
2407+
return;
2408+
}
2409+
} catch (RemoteException e) {
2410+
Slog.e(TAG, "Can't reach package manager", e);
2411+
return;
2412+
}
2413+
23992414
// no longer idle; we have backup work to do
24002415
unscheduleGcIdler();
24012416

24022417
// instantiate the BackupAgent class named in the manifest
24032418
LoadedApk packageInfo = getPackageInfoNoCheck(data.appInfo, data.compatInfo);
24042419
String packageName = packageInfo.mPackageName;
2420+
if (packageName == null) {
2421+
Slog.d(TAG, "Asked to create backup agent for nonexistent package");
2422+
return;
2423+
}
2424+
24052425
if (mBackupAgents.get(packageName) != null) {
24062426
Slog.d(TAG, "BackupAgent " + " for " + packageName
24072427
+ " already exists");

‎core/java/android/app/IActivityManager.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ public void serviceDoneExecuting(IBinder token, int type, int startId,
152152

153153
public boolean bindBackupAgent(ApplicationInfo appInfo, int backupRestoreMode)
154154
throws RemoteException;
155+
public void clearPendingBackup() throws RemoteException;
155156
public void backupAgentCreated(String packageName, IBinder agent) throws RemoteException;
156157
public void unbindBackupAgent(ApplicationInfo appInfo) throws RemoteException;
157158
public void killApplicationProcess(String processName, int uid) throws RemoteException;
@@ -619,4 +620,5 @@ private WaitResult(Parcel source) {
619620
int GET_RUNNING_USER_IDS_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+156;
620621
int REQUEST_BUG_REPORT_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+157;
621622
int INPUT_DISPATCHING_TIMED_OUT_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+158;
623+
int CLEAR_PENDING_BACKUP_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+159;
622624
}

‎services/java/com/android/server/BackupManagerService.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,6 +1474,7 @@ private void removePackageFromSetLocked(final HashSet<String> set,
14741474
if (MORE_DEBUG) Slog.v(TAG, " removing participant " + packageName);
14751475
removeEverBackedUp(packageName);
14761476
set.remove(packageName);
1477+
mPendingBackups.remove(packageName);
14771478
}
14781479
}
14791480

@@ -1625,13 +1626,15 @@ IBackupAgent bindToAgentSynchronous(ApplicationInfo app, int mode) {
16251626
} catch (InterruptedException e) {
16261627
// just bail
16271628
if (DEBUG) Slog.w(TAG, "Interrupted: " + e);
1629+
mActivityManager.clearPendingBackup();
16281630
return null;
16291631
}
16301632
}
16311633

16321634
// if we timed out with no connect, abort and move on
16331635
if (mConnecting == true) {
16341636
Slog.w(TAG, "Timeout waiting for agent " + app);
1637+
mActivityManager.clearPendingBackup();
16351638
return null;
16361639
}
16371640
if (DEBUG) Slog.i(TAG, "got agent " + mConnectedAgent);

‎services/java/com/android/server/am/ActivityManagerService.java‎

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11119,8 +11119,8 @@ public void serviceDoneExecuting(IBinder token, int type, int startId, int res)
1111911119
// instantiated. The backup agent will invoke backupAgentCreated() on the
1112011120
// activity manager to announce its creation.
1112111121
public boolean bindBackupAgent(ApplicationInfo app, int backupMode) {
11122-
if (DEBUG_BACKUP) Slog.v(TAG, "startBackupAgent: app=" + app + " mode=" + backupMode);
11123-
enforceCallingPermission("android.permission.BACKUP", "startBackupAgent");
11122+
if (DEBUG_BACKUP) Slog.v(TAG, "bindBackupAgent: app=" + app + " mode=" + backupMode);
11123+
enforceCallingPermission("android.permission.BACKUP", "bindBackupAgent");
1112411124

1112511125
synchronized(this) {
1112611126
// !!! TODO: currently no check here that we're already bound
@@ -11181,6 +11181,17 @@ public boolean bindBackupAgent(ApplicationInfo app, int backupMode) {
1118111181
return true;
1118211182
}
1118311183

11184+
@Override
11185+
public void clearPendingBackup() {
11186+
if (DEBUG_BACKUP) Slog.v(TAG, "clearPendingBackup");
11187+
enforceCallingPermission("android.permission.BACKUP", "clearPendingBackup");
11188+
11189+
synchronized (this) {
11190+
mBackupTarget = null;
11191+
mBackupAppName = null;
11192+
}
11193+
}
11194+
1118411195
// A backup agent has just come up
1118511196
public void backupAgentCreated(String agentPackageName, IBinder agent) {
1118611197
if (DEBUG_BACKUP) Slog.v(TAG, "backupAgentCreated: " + agentPackageName
@@ -11217,32 +11228,34 @@ public void unbindBackupAgent(ApplicationInfo appInfo) {
1121711228
}
1121811229

1121911230
synchronized(this) {
11220-
if (mBackupAppName == null) {
11221-
Slog.w(TAG, "Unbinding backup agent with no active backup");
11222-
return;
11223-
}
11224-
11225-
if (!mBackupAppName.equals(appInfo.packageName)) {
11226-
Slog.e(TAG, "Unbind of " + appInfo + " but is not the current backup target");
11227-
return;
11228-
}
11231+
try {
11232+
if (mBackupAppName == null) {
11233+
Slog.w(TAG, "Unbinding backup agent with no active backup");
11234+
return;
11235+
}
1122911236

11230-
ProcessRecord proc = mBackupTarget.app;
11231-
mBackupTarget = null;
11232-
mBackupAppName = null;
11237+
if (!mBackupAppName.equals(appInfo.packageName)) {
11238+
Slog.e(TAG, "Unbind of " + appInfo + " but is not the current backup target");
11239+
return;
11240+
}
1123311241

11234-
// Not backing this app up any more; reset its OOM adjustment
11235-
updateOomAdjLocked(proc);
11242+
// Not backing this app up any more; reset its OOM adjustment
11243+
final ProcessRecord proc = mBackupTarget.app;
11244+
updateOomAdjLocked(proc);
1123611245

11237-
// If the app crashed during backup, 'thread' will be null here
11238-
if (proc.thread != null) {
11239-
try {
11240-
proc.thread.scheduleDestroyBackupAgent(appInfo,
11241-
compatibilityInfoForPackageLocked(appInfo));
11242-
} catch (Exception e) {
11243-
Slog.e(TAG, "Exception when unbinding backup agent:");
11244-
e.printStackTrace();
11246+
// If the app crashed during backup, 'thread' will be null here
11247+
if (proc.thread != null) {
11248+
try {
11249+
proc.thread.scheduleDestroyBackupAgent(appInfo,
11250+
compatibilityInfoForPackageLocked(appInfo));
11251+
} catch (Exception e) {
11252+
Slog.e(TAG, "Exception when unbinding backup agent:");
11253+
e.printStackTrace();
11254+
}
1124511255
}
11256+
} finally {
11257+
mBackupTarget = null;
11258+
mBackupAppName = null;
1124611259
}
1124711260
}
1124811261
}

0 commit comments

Comments
 (0)