From a3c06bc6bb2fd72c282bea8f622df02a356c7820 Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Wed, 9 Jun 2021 16:31:11 +0100 Subject: Don't leak AccessibilityManager binder proxy. Bug: 190412311 Test: atest MixedDeviceOwnerTest#testPermittedAccessibilityServices Test: com.android.cts.devicepolicy.OrgOwnedProfileOwnerTest#testPersonalAppsSuspensionNormalApp Change-Id: I34b0ff5cbf52786e0f0c10f787e2ceddfca1e4e1 --- .../devicepolicy/DevicePolicyManagerService.java | 33 +++++++++++++--------- .../devicepolicy/PersonalAppsSuspensionHelper.java | 26 +++++++++-------- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 228bc0ecc051..a32bf76b2122 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -10005,14 +10005,23 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return true; } - private AccessibilityManager getAccessibilityManagerForUser(int userId) { + /** + * Invoke a method in AccessibilityManager ensuring the client is removed. + */ + private T withAccessibilityManager( + int userId, Function function) { // Not using AccessibilityManager.getInstance because that guesses // at the user you require based on callingUid and caches for a given // process. - IBinder iBinder = ServiceManager.getService(Context.ACCESSIBILITY_SERVICE); - IAccessibilityManager service = iBinder == null + final IBinder iBinder = ServiceManager.getService(Context.ACCESSIBILITY_SERVICE); + final IAccessibilityManager service = iBinder == null ? null : IAccessibilityManager.Stub.asInterface(iBinder); - return new AccessibilityManager(mContext, service, userId); + final AccessibilityManager am = new AccessibilityManager(mContext, service, userId); + try { + return function.apply(am); + } finally { + am.removeClient(); + } } @Override @@ -10025,22 +10034,21 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (packageList != null) { int userId = caller.getUserId(); - List enabledServices = null; + final List enabledServices; long id = mInjector.binderClearCallingIdentity(); try { UserInfo user = getUserInfo(userId); if (user.isManagedProfile()) { userId = user.profileGroupId; } - AccessibilityManager accessibilityManager = getAccessibilityManagerForUser(userId); - enabledServices = accessibilityManager.getEnabledAccessibilityServiceList( - FEEDBACK_ALL_MASK); + enabledServices = withAccessibilityManager(userId, + am -> am.getEnabledAccessibilityServiceList(FEEDBACK_ALL_MASK)); } finally { mInjector.binderRestoreCallingIdentity(id); } if (enabledServices != null) { - List enabledPackages = new ArrayList(); + List enabledPackages = new ArrayList<>(); for (AccessibilityServiceInfo service : enabledServices) { enabledPackages.add(service.getResolveInfo().serviceInfo.packageName); } @@ -10122,10 +10130,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (user.isManagedProfile()) { userId = user.profileGroupId; } - AccessibilityManager accessibilityManager = - getAccessibilityManagerForUser(userId); - List installedServices = - accessibilityManager.getInstalledAccessibilityServiceList(); + final List installedServices = + withAccessibilityManager(userId, + AccessibilityManager::getInstalledAccessibilityServiceList); if (installedServices != null) { for (AccessibilityServiceInfo service : installedServices) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/PersonalAppsSuspensionHelper.java b/services/devicepolicy/java/com/android/server/devicepolicy/PersonalAppsSuspensionHelper.java index 48d2d73543fb..532823ad8367 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/PersonalAppsSuspensionHelper.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/PersonalAppsSuspensionHelper.java @@ -18,8 +18,6 @@ package com.android.server.devicepolicy; import static android.accessibilityservice.AccessibilityServiceInfo.FEEDBACK_ALL_MASK; -import static com.android.server.devicepolicy.DevicePolicyManagerService.LOG_TAG; - import android.accessibilityservice.AccessibilityServiceInfo; import android.annotation.Nullable; import android.annotation.UserIdInt; @@ -142,9 +140,21 @@ public final class PersonalAppsSuspensionHelper { } private List getAccessibilityServices() { - final List accessibilityServiceInfos = - getAccessibilityManagerForUser(mContext.getUserId()) - .getEnabledAccessibilityServiceList(FEEDBACK_ALL_MASK); + final List accessibilityServiceInfos; + // Not using AccessibilityManager.getInstance because that guesses + // at the user you require based on callingUid and caches for a given + // process. + final IBinder iBinder = ServiceManager.getService(Context.ACCESSIBILITY_SERVICE); + final IAccessibilityManager service = iBinder == null + ? null : IAccessibilityManager.Stub.asInterface(iBinder); + final AccessibilityManager am = + new AccessibilityManager(mContext, service, mContext.getUserId()); + try { + accessibilityServiceInfos = am.getEnabledAccessibilityServiceList(FEEDBACK_ALL_MASK); + } finally { + am.removeClient(); + } + final List result = new ArrayList<>(); for (final AccessibilityServiceInfo serviceInfo : accessibilityServiceInfos) { final ComponentName componentName = @@ -192,12 +202,6 @@ public final class PersonalAppsSuspensionHelper { return resolveInfos != null && !resolveInfos.isEmpty(); } - private AccessibilityManager getAccessibilityManagerForUser(int userId) { - final IBinder iBinder = ServiceManager.getService(Context.ACCESSIBILITY_SERVICE); - final IAccessibilityManager service = - iBinder == null ? null : IAccessibilityManager.Stub.asInterface(iBinder); - return new AccessibilityManager(mContext, service, userId); - } void dump(IndentingPrintWriter pw) { pw.println("PersonalAppsSuspensionHelper"); -- cgit v1.2.3-59-g8ed1b