Don't kill for REQUEST_INSTALL_PACKAGES on default/error mode transition.

When handling unknown sources, PackageManager transitions from
MODE_DEFAULT to MODE_ERRORED, causing us to kill the app before the user
has even decided whether to allow external sources or not.

Since MODE_DEFAULT and MODE_ERRORED are the same from a storage point of
view, ignore transitions between the two.

This required some AppOps changes to store the previous mode and pass it
in.

Bug: 162849988
Test: run Epic installer
Change-Id: Ic866216f877e9b727fe70556f66dd998966fe0a2
This commit is contained in:
Martijn Coenen
2020-09-08 14:39:38 +02:00
parent d5283affca
commit e9e278f670
3 changed files with 37 additions and 20 deletions

View File

@@ -118,7 +118,7 @@ public abstract class StorageManagerInternal {
* affects them. * affects them.
*/ */
public abstract void onAppOpsChanged(int code, int uid, public abstract void onAppOpsChanged(int code, int uid,
@Nullable String packageName, int mode); @Nullable String packageName, int mode, int previousMode);
/** /**
* Asks the StorageManager to reset all state for the provided user; this will result * Asks the StorageManager to reset all state for the provided user; this will result

View File

@@ -4687,14 +4687,19 @@ class StorageManagerService extends IStorageManager.Stub
} }
} }
public void onAppOpsChanged(int code, int uid, @Nullable String packageName, int mode) { public void onAppOpsChanged(int code, int uid, @Nullable String packageName, int mode,
int previousMode) {
final long token = Binder.clearCallingIdentity(); final long token = Binder.clearCallingIdentity();
try { try {
// When using FUSE, we may need to kill the app if the op changes // When using FUSE, we may need to kill the app if the op changes
switch(code) { switch(code) {
case OP_REQUEST_INSTALL_PACKAGES: case OP_REQUEST_INSTALL_PACKAGES:
// Always kill regardless of op change, to remount apps /storage if (previousMode == MODE_ALLOWED || mode == MODE_ALLOWED) {
killAppForOpChange(code, uid); // If we transition to/from MODE_ALLOWED, kill the app to make
// sure it has the correct view of /storage. Changing between
// MODE_DEFAULT / MODE_ERRORED is a no-op
killAppForOpChange(code, uid);
}
return; return;
case OP_MANAGE_EXTERNAL_STORAGE: case OP_MANAGE_EXTERNAL_STORAGE:
if (mode != MODE_ALLOWED) { if (mode != MODE_ALLOWED) {

View File

@@ -2206,6 +2206,7 @@ public class AppOpsService extends IAppOpsService.Stub {
updatePermissionRevokedCompat(uid, code, mode); updatePermissionRevokedCompat(uid, code, mode);
} }
int previousMode;
synchronized (this) { synchronized (this) {
final int defaultMode = AppOpsManager.opToDefaultMode(code); final int defaultMode = AppOpsManager.opToDefaultMode(code);
@@ -2214,12 +2215,14 @@ public class AppOpsService extends IAppOpsService.Stub {
if (mode == defaultMode) { if (mode == defaultMode) {
return; return;
} }
previousMode = AppOpsManager.MODE_DEFAULT;
uidState = new UidState(uid); uidState = new UidState(uid);
uidState.opModes = new SparseIntArray(); uidState.opModes = new SparseIntArray();
uidState.opModes.put(code, mode); uidState.opModes.put(code, mode);
mUidStates.put(uid, uidState); mUidStates.put(uid, uidState);
scheduleWriteLocked(); scheduleWriteLocked();
} else if (uidState.opModes == null) { } else if (uidState.opModes == null) {
previousMode = AppOpsManager.MODE_DEFAULT;
if (mode != defaultMode) { if (mode != defaultMode) {
uidState.opModes = new SparseIntArray(); uidState.opModes = new SparseIntArray();
uidState.opModes.put(code, mode); uidState.opModes.put(code, mode);
@@ -2229,6 +2232,7 @@ public class AppOpsService extends IAppOpsService.Stub {
if (uidState.opModes.indexOfKey(code) >= 0 && uidState.opModes.get(code) == mode) { if (uidState.opModes.indexOfKey(code) >= 0 && uidState.opModes.get(code) == mode) {
return; return;
} }
previousMode = uidState.opModes.get(code);
if (mode == defaultMode) { if (mode == defaultMode) {
uidState.opModes.delete(code); uidState.opModes.delete(code);
if (uidState.opModes.size() <= 0) { if (uidState.opModes.size() <= 0) {
@@ -2243,7 +2247,7 @@ public class AppOpsService extends IAppOpsService.Stub {
} }
notifyOpChangedForAllPkgsInUid(code, uid, false, permissionPolicyCallback); notifyOpChangedForAllPkgsInUid(code, uid, false, permissionPolicyCallback);
notifyOpChangedSync(code, uid, null, mode); notifyOpChangedSync(code, uid, null, mode, previousMode);
} }
/** /**
@@ -2420,11 +2424,12 @@ public class AppOpsService extends IAppOpsService.Stub {
} }
} }
private void notifyOpChangedSync(int code, int uid, @NonNull String packageName, int mode) { private void notifyOpChangedSync(int code, int uid, @NonNull String packageName, int mode,
int previousMode) {
final StorageManagerInternal storageManagerInternal = final StorageManagerInternal storageManagerInternal =
LocalServices.getService(StorageManagerInternal.class); LocalServices.getService(StorageManagerInternal.class);
if (storageManagerInternal != null) { if (storageManagerInternal != null) {
storageManagerInternal.onAppOpsChanged(code, uid, packageName, mode); storageManagerInternal.onAppOpsChanged(code, uid, packageName, mode, previousMode);
} }
} }
@@ -2458,11 +2463,13 @@ public class AppOpsService extends IAppOpsService.Stub {
return; return;
} }
int previousMode = AppOpsManager.MODE_DEFAULT;
synchronized (this) { synchronized (this) {
UidState uidState = getUidStateLocked(uid, false); UidState uidState = getUidStateLocked(uid, false);
Op op = getOpLocked(code, uid, packageName, null, bypass, true); Op op = getOpLocked(code, uid, packageName, null, bypass, true);
if (op != null) { if (op != null) {
if (op.mode != mode) { if (op.mode != mode) {
previousMode = op.mode;
op.mode = mode; op.mode = mode;
if (uidState != null) { if (uidState != null) {
uidState.evalForegroundOps(mOpModeWatchers); uidState.evalForegroundOps(mOpModeWatchers);
@@ -2499,7 +2506,7 @@ public class AppOpsService extends IAppOpsService.Stub {
this, repCbs, code, uid, packageName)); this, repCbs, code, uid, packageName));
} }
notifyOpChangedSync(code, uid, packageName, mode); notifyOpChangedSync(code, uid, packageName, mode, previousMode);
} }
private void notifyOpChanged(ArraySet<ModeCallback> callbacks, int code, private void notifyOpChanged(ArraySet<ModeCallback> callbacks, int code,
@@ -2542,7 +2549,7 @@ public class AppOpsService extends IAppOpsService.Stub {
} }
private static ArrayList<ChangeRec> addChange(ArrayList<ChangeRec> reports, private static ArrayList<ChangeRec> addChange(ArrayList<ChangeRec> reports,
int op, int uid, String packageName) { int op, int uid, String packageName, int previousMode) {
boolean duplicate = false; boolean duplicate = false;
if (reports == null) { if (reports == null) {
reports = new ArrayList<>(); reports = new ArrayList<>();
@@ -2557,7 +2564,7 @@ public class AppOpsService extends IAppOpsService.Stub {
} }
} }
if (!duplicate) { if (!duplicate) {
reports.add(new ChangeRec(op, uid, packageName)); reports.add(new ChangeRec(op, uid, packageName, previousMode));
} }
return reports; return reports;
@@ -2565,7 +2572,7 @@ public class AppOpsService extends IAppOpsService.Stub {
private static HashMap<ModeCallback, ArrayList<ChangeRec>> addCallbacks( private static HashMap<ModeCallback, ArrayList<ChangeRec>> addCallbacks(
HashMap<ModeCallback, ArrayList<ChangeRec>> callbacks, HashMap<ModeCallback, ArrayList<ChangeRec>> callbacks,
int op, int uid, String packageName, ArraySet<ModeCallback> cbs) { int op, int uid, String packageName, int previousMode, ArraySet<ModeCallback> cbs) {
if (cbs == null) { if (cbs == null) {
return callbacks; return callbacks;
} }
@@ -2576,7 +2583,7 @@ public class AppOpsService extends IAppOpsService.Stub {
for (int i=0; i<N; i++) { for (int i=0; i<N; i++) {
ModeCallback cb = cbs.valueAt(i); ModeCallback cb = cbs.valueAt(i);
ArrayList<ChangeRec> reports = callbacks.get(cb); ArrayList<ChangeRec> reports = callbacks.get(cb);
ArrayList<ChangeRec> changed = addChange(reports, op, uid, packageName); ArrayList<ChangeRec> changed = addChange(reports, op, uid, packageName, previousMode);
if (changed != reports) { if (changed != reports) {
callbacks.put(cb, changed); callbacks.put(cb, changed);
} }
@@ -2588,11 +2595,13 @@ public class AppOpsService extends IAppOpsService.Stub {
final int op; final int op;
final int uid; final int uid;
final String pkg; final String pkg;
final int previous_mode;
ChangeRec(int _op, int _uid, String _pkg) { ChangeRec(int _op, int _uid, String _pkg, int _previous_mode) {
op = _op; op = _op;
uid = _uid; uid = _uid;
pkg = _pkg; pkg = _pkg;
previous_mode = _previous_mode;
} }
} }
@@ -2628,18 +2637,19 @@ public class AppOpsService extends IAppOpsService.Stub {
for (int j = uidOpCount - 1; j >= 0; j--) { for (int j = uidOpCount - 1; j >= 0; j--) {
final int code = opModes.keyAt(j); final int code = opModes.keyAt(j);
if (AppOpsManager.opAllowsReset(code)) { if (AppOpsManager.opAllowsReset(code)) {
int previousMode = opModes.valueAt(j);
opModes.removeAt(j); opModes.removeAt(j);
if (opModes.size() <= 0) { if (opModes.size() <= 0) {
uidState.opModes = null; uidState.opModes = null;
} }
for (String packageName : getPackagesForUid(uidState.uid)) { for (String packageName : getPackagesForUid(uidState.uid)) {
callbacks = addCallbacks(callbacks, code, uidState.uid, packageName, callbacks = addCallbacks(callbacks, code, uidState.uid, packageName,
mOpModeWatchers.get(code)); previousMode, mOpModeWatchers.get(code));
callbacks = addCallbacks(callbacks, code, uidState.uid, packageName, callbacks = addCallbacks(callbacks, code, uidState.uid, packageName,
mPackageModeWatchers.get(packageName)); previousMode, mPackageModeWatchers.get(packageName));
allChanges = addChange(allChanges, code, uidState.uid, allChanges = addChange(allChanges, code, uidState.uid,
packageName); packageName, previousMode);
} }
} }
} }
@@ -2670,16 +2680,18 @@ public class AppOpsService extends IAppOpsService.Stub {
Op curOp = pkgOps.valueAt(j); Op curOp = pkgOps.valueAt(j);
if (AppOpsManager.opAllowsReset(curOp.op) if (AppOpsManager.opAllowsReset(curOp.op)
&& curOp.mode != AppOpsManager.opToDefaultMode(curOp.op)) { && curOp.mode != AppOpsManager.opToDefaultMode(curOp.op)) {
int previousMode = curOp.mode;
curOp.mode = AppOpsManager.opToDefaultMode(curOp.op); curOp.mode = AppOpsManager.opToDefaultMode(curOp.op);
changed = true; changed = true;
uidChanged = true; uidChanged = true;
final int uid = curOp.uidState.uid; final int uid = curOp.uidState.uid;
callbacks = addCallbacks(callbacks, curOp.op, uid, packageName, callbacks = addCallbacks(callbacks, curOp.op, uid, packageName,
mOpModeWatchers.get(curOp.op)); previousMode, mOpModeWatchers.get(curOp.op));
callbacks = addCallbacks(callbacks, curOp.op, uid, packageName, callbacks = addCallbacks(callbacks, curOp.op, uid, packageName,
mPackageModeWatchers.get(packageName)); previousMode, mPackageModeWatchers.get(packageName));
allChanges = addChange(allChanges, curOp.op, uid, packageName); allChanges = addChange(allChanges, curOp.op, uid, packageName,
previousMode);
curOp.removeAttributionsWithNoTime(); curOp.removeAttributionsWithNoTime();
if (curOp.mAttributions.isEmpty()) { if (curOp.mAttributions.isEmpty()) {
pkgOps.removeAt(j); pkgOps.removeAt(j);
@@ -2720,7 +2732,7 @@ public class AppOpsService extends IAppOpsService.Stub {
for (int i = 0; i < numChanges; i++) { for (int i = 0; i < numChanges; i++) {
ChangeRec change = allChanges.get(i); ChangeRec change = allChanges.get(i);
notifyOpChangedSync(change.op, change.uid, change.pkg, notifyOpChangedSync(change.op, change.uid, change.pkg,
AppOpsManager.opToDefaultMode(change.op)); AppOpsManager.opToDefaultMode(change.op), change.previous_mode);
} }
} }
} }