Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5b43fca0 authored by Phil Weaver's avatar Phil Weaver
Browse files

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
parent c01dd791

services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java

+7 −11
Original line number Diff line number Diff line
@@ -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,25 +573,25 @@ 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;

        }

    }



    @Override

    public void interrupt(int userId) {