Nuanced Uri handling related to WM/AM locking.
As part of detangling WM/AM locks, we added a Slog.wtf() to identify
and lurking cases where we tried resolveActivity() with the WM lock
held. This helped us uncover startActivityFromRecents(), but the
logic there is unfortunately too risky to refactor.
Instead, this change narrows our locking detection to only consider
cases where we'd actually risk a deadlock; that is, when the thread
is holding the WM lock and we're just about to acquire the AM lock.
This change now avoids deadlock completely by assuming that we can't
check Uri permissions in these cases where we know a deadlock is
imminent; we instead use a safe-default that the Uri permission is
denied, and Slog.wtf() to identify which paths are triggering it.
The only path we've identified so far that triggers this logic is
startActivityFromRecents() which can be reproduced by re-launching
a recent task after a device reboot, which when combined with Uris
that require "forceUriPermissions" is a very rare situation.
Bug: 159937485, 115619667
Test: manual
Change-Id: I4ed1c402703e0772c0164ac0acc64f3a754512f3
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index e77b361..c9dbacd 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -8086,6 +8086,12 @@
}
int checkContentProviderUriPermission(Uri uri, int userId, int callingUid, int modeFlags) {
+ if (Thread.holdsLock(mActivityTaskManager.getGlobalLock())) {
+ Slog.wtf(TAG, new IllegalStateException("Unable to check Uri permission"
+ + " because caller is holding WM lock; assuming permission denied"));
+ return PackageManager.PERMISSION_DENIED;
+ }
+
final String name = uri.getAuthority();
final long ident = Binder.clearCallingIdentity();
ContentProviderHolder holder = null;
diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java
index 31712ef..b378621 100644
--- a/services/core/java/com/android/server/wm/ActivityStarter.java
+++ b/services/core/java/com/android/server/wm/ActivityStarter.java
@@ -640,13 +640,10 @@
}
// If the caller hasn't already resolved the activity, we're willing
- // to do so here, but because that may require acquiring the AM lock
- // as part of calculating the NeededUriGrants, we must never hold
- // the WM lock here to avoid deadlocking.
+ // to do so here. If the caller is already holding the WM lock here,
+ // and we need to check dynamic Uri permissions, then we're forced
+ // to assume those permissions are denied to avoid deadlocking.
if (mRequest.activityInfo == null) {
- if (Thread.holdsLock(mService.mGlobalLock)) {
- Slog.wtf(TAG, new IllegalStateException("Caller must not hold WM lock"));
- }
mRequest.resolveActivity(mSupervisor);
}
