From 42e44711fa5e5ba20604d654ae74b22dd73b3a40 Mon Sep 17 00:00:00 2001
From: Jing Ji <jji@google.com>
Date: Thu, 3 Jun 2021 12:29:31 -0700
Subject: [PATCH] Fix a race condition in acquiring content provider

Client could be stuck at the lock before toggling the waiting flag,
meanwhile the publishing of the provider could been holding the lock
and check that flag, thus result in not notifying the client.

Now setting the flag with the global lock held.

Bug: 186228106
Bug: 190034355
Test: atest CtsContentTestCases:android.content.cts
Test: atest FrameworksCoreTests:android.content
Change-Id: I7c7a3326c302cc01e52a9a5d78ea9d089be0dcae
---
 .../server/am/ContentProviderHelper.java      | 53 ++++++++++---------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/services/core/java/com/android/server/am/ContentProviderHelper.java b/services/core/java/com/android/server/am/ContentProviderHelper.java
index 795cd0a8bda78..d713834e91de4 100644
--- a/services/core/java/com/android/server/am/ContentProviderHelper.java
+++ b/services/core/java/com/android/server/am/ContentProviderHelper.java
@@ -501,37 +501,38 @@ public class ContentProviderHelper {
 
             mService.grantImplicitAccess(userId, null, callingUid,
                     UserHandle.getAppId(cpi.applicationInfo.uid));
-        }
 
-        if (caller != null) {
-            // The client will be waiting, and we'll notify it when the provider is ready.
-            synchronized (cpr) {
-                if (cpr.provider == null) {
-                    if (cpr.launchingApp == null) {
-                        Slog.w(TAG, "Unable to launch app "
-                                + cpi.applicationInfo.packageName + "/"
-                                + cpi.applicationInfo.uid + " for provider "
-                                + name + ": launching app became null");
-                        EventLogTags.writeAmProviderLostProcess(
-                                UserHandle.getUserId(cpi.applicationInfo.uid),
-                                cpi.applicationInfo.packageName,
-                                cpi.applicationInfo.uid, name);
-                        return null;
-                    }
+            if (caller != null) {
+                // The client will be waiting, and we'll notify it when the provider is ready.
+                synchronized (cpr) {
+                    if (cpr.provider == null) {
+                        if (cpr.launchingApp == null) {
+                            Slog.w(TAG, "Unable to launch app "
+                                    + cpi.applicationInfo.packageName + "/"
+                                    + cpi.applicationInfo.uid + " for provider "
+                                    + name + ": launching app became null");
+                            EventLogTags.writeAmProviderLostProcess(
+                                    UserHandle.getUserId(cpi.applicationInfo.uid),
+                                    cpi.applicationInfo.packageName,
+                                    cpi.applicationInfo.uid, name);
+                            return null;
+                        }
 
-                    if (conn != null) {
-                        conn.waiting = true;
+                        if (conn != null) {
+                            conn.waiting = true;
+                        }
+                        Message msg = mService.mHandler.obtainMessage(
+                                ActivityManagerService.WAIT_FOR_CONTENT_PROVIDER_TIMEOUT_MSG);
+                        msg.obj = cpr;
+                        mService.mHandler.sendMessageDelayed(msg,
+                                ContentResolver.CONTENT_PROVIDER_READY_TIMEOUT_MILLIS);
                     }
-                    Message msg = mService.mHandler.obtainMessage(
-                            ActivityManagerService.WAIT_FOR_CONTENT_PROVIDER_TIMEOUT_MSG);
-                    msg.obj = cpr;
-                    mService.mHandler.sendMessageDelayed(msg,
-                            ContentResolver.CONTENT_PROVIDER_READY_TIMEOUT_MILLIS);
                 }
+                // Return a holder instance even if we are waiting for the publishing of the
+                // provider, client will check for the holder.provider to see if it needs to wait
+                // for it.
+                return cpr.newHolder(conn, false);
             }
-            // Return a holder instance even if we are waiting for the publishing of the provider,
-            // client will check for the holder.provider to see if it needs to wait for it.
-            return cpr.newHolder(conn, false);
         }
 
         // Because of the provider's external client (i.e., SHELL), we'll have to wait right here.