From eaa7835d03348af38535ce6a835408a63cee7b87 Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Fri, 7 May 2021 03:19:14 -0700 Subject: [PATCH] Fix issue with acquiring singleton content provider from another user The content provider host app could be living in a different user from the client app's expectation when it's singleton, this could result in client app ignores notification of the start of the content provider because of the discrepancy of user id. Now remember the original expected user id so client app can clearly know whether the incoming notification of the start of a content provider is what it expects or not. Bug: 181789697 Test: atest CtsContentTestCases:android.content.cts Test: atest FrameworksCoreTests:android.content Test: Manual - steps in b/181789697#comment1 Change-Id: Ic909620059e1f989a5e7c8cf3ede2b8d63814a96 --- .../android/server/am/ContentProviderConnection.java | 9 ++++++++- .../com/android/server/am/ContentProviderHelper.java | 11 +++++++---- .../com/android/server/am/ContentProviderRecord.java | 5 ++--- .../android/server/am/MockingOomAdjusterTests.java | 2 +- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/am/ContentProviderConnection.java b/services/core/java/com/android/server/am/ContentProviderConnection.java index efee4321fd9fb..3bc4fcf089212 100644 --- a/services/core/java/com/android/server/am/ContentProviderConnection.java +++ b/services/core/java/com/android/server/am/ContentProviderConnection.java @@ -19,6 +19,7 @@ package com.android.server.am; import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_PROVIDER; import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM; +import android.annotation.UserIdInt; import android.os.Binder; import android.os.SystemClock; import android.util.Slog; @@ -52,15 +53,21 @@ public final class ContentProviderConnection extends Binder { // The provider of this connection is now dead. public boolean dead; + // The original user id when this connection was requested, it could be different from + // the client's user id because the client could request to access a content provider + // living in a different user if it has the permission. + @UserIdInt final int mExpectedUserId; + // For debugging. private int mNumStableIncs; private int mNumUnstableIncs; public ContentProviderConnection(ContentProviderRecord _provider, ProcessRecord _client, - String _clientPackage) { + String _clientPackage, @UserIdInt int _expectedUserId) { provider = _provider; client = _client; clientPackage = _clientPackage; + mExpectedUserId = _expectedUserId; createTime = SystemClock.elapsedRealtime(); } diff --git a/services/core/java/com/android/server/am/ContentProviderHelper.java b/services/core/java/com/android/server/am/ContentProviderHelper.java index fb9f2dd58c58a..795cd0a8bda78 100644 --- a/services/core/java/com/android/server/am/ContentProviderHelper.java +++ b/services/core/java/com/android/server/am/ContentProviderHelper.java @@ -150,6 +150,7 @@ public class ContentProviderHelper { ContentProviderConnection conn = null; ProviderInfo cpi = null; boolean providerRunning = false; + final int expectedUserId = userId; synchronized (mService) { long startTime = SystemClock.uptimeMillis(); @@ -239,7 +240,8 @@ public class ContentProviderHelper { // Return the provider instance right away since it already exists. conn = incProviderCountLocked(r, cpr, token, callingUid, callingPackage, - callingTag, stable, true, startTime, mService.mProcessList); + callingTag, stable, true, startTime, mService.mProcessList, + expectedUserId); checkTime(startTime, "getContentProviderImpl: before updateOomAdj"); final int verifiedAdj = cpr.proc.mState.getVerifiedAdj(); @@ -490,7 +492,7 @@ public class ContentProviderHelper { mProviderMap.putProviderByName(name, cpr); conn = incProviderCountLocked(r, cpr, token, callingUid, callingPackage, callingTag, - stable, false, startTime, mService.mProcessList); + stable, false, startTime, mService.mProcessList, expectedUserId); if (conn != null) { conn.waiting = true; } @@ -1286,7 +1288,7 @@ public class ContentProviderHelper { private ContentProviderConnection incProviderCountLocked(ProcessRecord r, final ContentProviderRecord cpr, IBinder externalProcessToken, int callingUid, String callingPackage, String callingTag, boolean stable, boolean updateLru, - long startTime, ProcessList processList) { + long startTime, ProcessList processList, @UserIdInt int expectedUserId) { if (r == null) { cpr.addExternalProcessHandleLocked(externalProcessToken, callingUid, callingTag); return null; @@ -1303,7 +1305,8 @@ public class ContentProviderHelper { } // Create a new ContentProviderConnection. The reference count is known to be 1. - ContentProviderConnection conn = new ContentProviderConnection(cpr, r, callingPackage); + ContentProviderConnection conn = new ContentProviderConnection(cpr, r, callingPackage, + expectedUserId); conn.startAssociationIfNeeded(); conn.initializeCount(stable); cpr.connections.add(conn); diff --git a/services/core/java/com/android/server/am/ContentProviderRecord.java b/services/core/java/com/android/server/am/ContentProviderRecord.java index b217cae61d4cb..5fd15db79d660 100644 --- a/services/core/java/com/android/server/am/ContentProviderRecord.java +++ b/services/core/java/com/android/server/am/ContentProviderRecord.java @@ -189,7 +189,6 @@ final class ContentProviderRecord implements ComponentName.WithComponentName { */ void onProviderPublishStatusLocked(boolean status) { final int numOfConns = connections.size(); - final int userId = UserHandle.getUserId(appInfo.uid); for (int i = 0; i < numOfConns; i++) { final ContentProviderConnection conn = connections.get(i); if (conn.waiting && conn.client != null) { @@ -201,7 +200,7 @@ final class ContentProviderRecord implements ComponentName.WithComponentName { + appInfo.uid + " for provider " + info.authority + ": launching app became null"); EventLogTags.writeAmProviderLostProcess( - userId, + UserHandle.getUserId(appInfo.uid), appInfo.packageName, appInfo.uid, info.authority); } else { @@ -217,7 +216,7 @@ final class ContentProviderRecord implements ComponentName.WithComponentName { try { thread.notifyContentProviderPublishStatus( newHolder(status ? conn : null, false), - info.authority, userId, status); + info.authority, conn.mExpectedUserId, status); } catch (RemoteException e) { } } diff --git a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java index 3404aff976d0a..f59146e708410 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java @@ -2078,7 +2078,7 @@ public class MockingOomAdjusterTests { doReturn(hasExternalProviders).when(record).hasExternalProcessHandles(); } ContentProviderConnection conn = spy(new ContentProviderConnection(record, client, - client.info.packageName)); + client.info.packageName, UserHandle.getUserId(client.uid))); record.connections.add(conn); client.mProviders.addProviderConnection(conn); return record;