Fix crash when using list of enabled a11y services

AccessibilityManagerService#getEnabledAccessibilityServiceList
had an optimization to always return the same, statically
allocated object. This is almost safe if it's being called
from another process, as Binder will copy it. When called from
the same process, however, it's a lot less safe and seems to
have caused a crash.

I think the optimization was already problematic in Binder
calls as well, though. The method grabs a lock, but I think
it's still possible for another thread to call the method
while Binder is making the copy.

I'm removing the optimization and just allocating a new List
to prevent such crashes.

Bug: 36364829
Test: No repro case, so I just ran a11y CTS and unit tests.
Change-Id: Ib64703a7bbed82c6ca000d8703d23819188b4b9b

1 files changed, 7 insertions, 11 deletions

diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
index 1968d2e925aa..a57daabf5145 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
@@ -183,9 +183,6 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
     private final SimpleStringSplitter mStringColonSplitter =
             new SimpleStringSplitter(COMPONENT_NAME_SEPARATOR);
 
-    private final List<AccessibilityServiceInfo> mEnabledServicesForFeedbackTempList =
-            new ArrayList<>();
-
     private final Rect mTempRect = new Rect();
 
     private final Rect mTempRect1 = new Rect();
@@ -568,7 +565,6 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
     @Override
     public List<AccessibilityServiceInfo> getEnabledAccessibilityServiceList(int feedbackType,
             int userId) {
-        List<AccessibilityServiceInfo> result = null;
         synchronized (mLock) {
             // We treat calls from a profile as if made by its parent as profiles
             // share the accessibility state of the parent. The call below
@@ -577,24 +573,24 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub {
                     .resolveCallingUserIdEnforcingPermissionsLocked(userId);
 
             // The automation service can suppress other services.
-            UserState userState = getUserStateLocked(resolvedUserId);
+            final UserState userState = getUserStateLocked(resolvedUserId);
             if (userState.isUiAutomationSuppressingOtherServices()) {
                 return Collections.emptyList();
             }
 
-            result = mEnabledServicesForFeedbackTempList;
-            result.clear();
-            List<Service> services = userState.mBoundServices;
-            for (int serviceCount = services.size(), i = 0; i < serviceCount; ++i) {
-                Service service = services.get(i);
+            final List<Service> services = userState.mBoundServices;
+            final int serviceCount = services.size();
+            final List<AccessibilityServiceInfo> result = new ArrayList<>(serviceCount);
+            for (int i = 0; i < serviceCount; ++i) {
+                final Service service = services.get(i);
                 // Don't report the UIAutomation (fake service)
                 if (!sFakeAccessibilityServiceComponentName.equals(service.mComponentName)
                         && (service.mFeedbackType & feedbackType) != 0) {
                     result.add(service.mAccessibilityServiceInfo);
                 }
             }
+            return result;
         }
-        return result;
     }
 
     @Override