From 4b4918782cce985afaf9f4181067f3959ff00fbf Mon Sep 17 00:00:00 2001 From: Patrick Baumann Date: Tue, 25 Feb 2020 11:51:44 -0800 Subject: [PATCH] Logs for test & debug blocks This change adds logging for debuggable and test-only apps when they are the caller and visibility of another app is blocked due to app enumeration. It also adds an adb command to turn logging on and off for other apps to help developers while debugging issues. Test: atest AppsFilterTest AppEnumerationTests PackageManagerPerfTest Bug: 145623959 Change-Id: I1fa930ef40bf08b00c41f51aa25c50b2189395bf --- .../android/content/pm/IPackageManager.aidl | 1 - .../content/pm/PackageManagerInternal.java | 7 ++ .../com/android/server/pm/AppsFilter.java | 111 ++++++++++++------ .../server/pm/PackageManagerService.java | 12 ++ .../server/pm/PackageManagerShellCommand.java | 37 ++++++ 5 files changed, 131 insertions(+), 37 deletions(-) diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl index e9cdbf28e9cb2..8c3eef27dd580 100644 --- a/core/java/android/content/pm/IPackageManager.aidl +++ b/core/java/android/content/pm/IPackageManager.aidl @@ -748,5 +748,4 @@ interface IPackageManager { void clearMimeGroup(String packageName, String group); List getMimeGroup(String packageName, String group); - } diff --git a/services/core/java/android/content/pm/PackageManagerInternal.java b/services/core/java/android/content/pm/PackageManagerInternal.java index 485127a79c27a..31044d0f00851 100644 --- a/services/core/java/android/content/pm/PackageManagerInternal.java +++ b/services/core/java/android/content/pm/PackageManagerInternal.java @@ -986,4 +986,11 @@ public abstract class PackageManagerInternal { * Returns MIME types contained in {@code mimeGroup} from {@code packageName} package */ public abstract List getMimeGroup(String packageName, String mimeGroup); + + /** + * Toggles visibility logging to help in debugging the app enumeration feature. + * @param packageName the package name that should begin logging + * @param enabled true if visibility blocks should be logged + */ + public abstract void setVisibilityLogging(String packageName, boolean enabled); } diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java index 0ad0b2373a797..b90681de35182 100644 --- a/services/core/java/com/android/server/pm/AppsFilter.java +++ b/services/core/java/com/android/server/pm/AppsFilter.java @@ -43,6 +43,7 @@ import android.util.ArrayMap; import android.util.ArraySet; import android.util.Slog; import android.util.SparseArray; +import android.util.SparseBooleanArray; import android.util.SparseSetArray; import com.android.internal.R; @@ -123,6 +124,7 @@ public class AppsFilter { } public interface FeatureConfig { + /** Called when the system is ready and components can be queried. */ void onSystemReady(); @@ -132,11 +134,21 @@ public class AppsFilter { /** @return true if the feature is enabled for the given package. */ boolean packageIsEnabled(AndroidPackage pkg); + /** @return true if debug logging is enabled for the given package. */ + boolean isLoggingEnabled(int appId); + + /** + * Turns on logging for the given appId + * @param enable true if logging should be enabled, false if disabled. + */ + void enableLogging(int appId, boolean enable); + /** * Initializes the package enablement state for the given package. This gives opportunity * to do any expensive operations ahead of the actual checks. + * @param removed true if adding, false if removing */ - void initializePackageState(String packageName); + void updatePackageState(PackageSetting setting, boolean removed); } private static class FeatureConfigImpl implements FeatureConfig, CompatChange.ChangeListener { @@ -147,6 +159,9 @@ public class AppsFilter { PackageManager.APP_ENUMERATION_ENABLED_BY_DEFAULT; private final ArraySet mDisabledPackages = new ArraySet<>(); + @Nullable + private SparseBooleanArray mLoggingEnabled = null; + private FeatureConfigImpl( PackageManagerInternal pmInternal, PackageManagerService.Injector injector) { mPmInternal = pmInternal; @@ -192,39 +207,65 @@ public class AppsFilter { } } - private boolean fetchPackageIsEnabled(AndroidPackage pkg) { + @Override + public boolean isLoggingEnabled(int uid) { + return mLoggingEnabled != null && mLoggingEnabled.indexOfKey(uid) >= 0; + } + + @Override + public void enableLogging(int appId, boolean enable) { + if (enable) { + if (mLoggingEnabled == null) { + mLoggingEnabled = new SparseBooleanArray(); + } + mLoggingEnabled.put(appId, true); + } else { + if (mLoggingEnabled != null) { + final int index = mLoggingEnabled.indexOfKey(appId); + if (index >= 0) { + mLoggingEnabled.removeAt(index); + if (mLoggingEnabled.size() == 0) { + mLoggingEnabled = null; + } + } + } + } + } + + @Override + public void onCompatChange(String packageName) { + updateEnabledState(mPmInternal.getPackage(packageName)); + } + + private void updateEnabledState(AndroidPackage pkg) { final long token = Binder.clearCallingIdentity(); try { // TODO(b/135203078): Do not use toAppInfo - final boolean changeEnabled = + final boolean enabled = mInjector.getCompatibility().isChangeEnabled( PackageManager.FILTER_APPLICATION_QUERY, pkg.toAppInfoWithoutState()); - return changeEnabled; + if (enabled) { + mDisabledPackages.remove(pkg.getPackageName()); + } else { + mDisabledPackages.add(pkg.getPackageName()); + } } finally { Binder.restoreCallingIdentity(token); } } @Override - public void onCompatChange(String packageName) { - final AndroidPackage pkg = mPmInternal.getPackage(packageName); - if (pkg == null) { - mDisabledPackages.remove(packageName); - return; - } - boolean enabled = fetchPackageIsEnabled(pkg); - if (enabled) { - mDisabledPackages.remove(packageName); + public void updatePackageState(PackageSetting setting, boolean removed) { + final boolean enableLogging = + !removed && (setting.pkg.isTestOnly() || setting.pkg.isDebuggable()); + enableLogging(setting.appId, enableLogging); + if (removed) { + mDisabledPackages.remove(setting.pkg.getPackageName()); } else { - mDisabledPackages.add(packageName); + updateEnabledState(setting.pkg); } } - - @Override - public void initializePackageState(String packageName) { - onCompatChange(packageName); - } } /** Builder method for an AppsFilter */ @@ -250,6 +291,10 @@ public class AppsFilter { forceSystemAppsQueryable, null); } + public FeatureConfig getFeatureConfig() { + return mFeatureConfig; + } + /** Returns true if the querying package may query for the potential target package */ private static boolean canQueryViaComponents(AndroidPackage querying, AndroidPackage potentialTarget) { @@ -447,7 +492,7 @@ public class AppsFilter { } } mOverlayReferenceMapper.addPkg(newPkgSetting.pkg, existingPkgs); - mFeatureConfig.initializePackageState(newPkgSetting.pkg.getPackageName()); + mFeatureConfig.updatePackageState(newPkgSetting, false /*removed*/); } finally { Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); } @@ -499,7 +544,7 @@ public class AppsFilter { } mOverlayReferenceMapper.removePkg(setting.name); - mFeatureConfig.initializePackageState(setting.pkg.getPackageName()); + mFeatureConfig.updatePackageState(setting, true /*removed*/); } /** @@ -516,13 +561,13 @@ public class AppsFilter { PackageSetting targetPkgSetting, int userId) { Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "shouldFilterApplication"); try { - if (!shouldFilterApplicationInternal(callingUid, callingSetting, targetPkgSetting, - userId)) { + + if (!shouldFilterApplicationInternal( + callingUid, callingSetting, targetPkgSetting, userId)) { return false; } - if (DEBUG_LOGGING) { - log(callingSetting, targetPkgSetting, - DEBUG_ALLOW_ALL ? "ALLOWED" : "BLOCKED", new RuntimeException()); + if (DEBUG_LOGGING || mFeatureConfig.isLoggingEnabled(UserHandle.getAppId(callingUid))) { + log(callingSetting, targetPkgSetting, "BLOCKED"); } return !DEBUG_ALLOW_ALL; } finally { @@ -737,17 +782,11 @@ public class AppsFilter { } } - private static void log(SettingBase callingPkgSetting, PackageSetting targetPkgSetting, + private static void log(SettingBase callingSetting, PackageSetting targetPkgSetting, String description) { - log(callingPkgSetting, targetPkgSetting, description, null); - } - - private static void log(SettingBase callingPkgSetting, PackageSetting targetPkgSetting, - String description, Throwable throwable) { - Slog.wtf(TAG, - "interaction: " + callingPkgSetting - + " -> " + targetPkgSetting + " " - + description, throwable); + Slog.i(TAG, + "interaction: " + (callingSetting == null ? "system" : callingSetting) + " -> " + + targetPkgSetting + " " + description); } public void dumpQueries( diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index a8996d5cfea42..576919f24a4cd 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -24152,6 +24152,18 @@ public class PackageManagerService extends IPackageManager.Stub public List getMimeGroup(String packageName, String mimeGroup) { return PackageManagerService.this.getMimeGroup(packageName, mimeGroup); } + + @Override + public void setVisibilityLogging(String packageName, boolean enable) { + final PackageSetting pkg; + synchronized (mLock) { + pkg = mSettings.getPackageLPr(packageName); + } + if (pkg == null) { + throw new IllegalStateException("No package found for " + packageName); + } + mAppsFilter.getFeatureConfig().enableLogging(pkg.appId, enable); + } } @GuardedBy("mLock") diff --git a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java index 0f06c186b9ff9..be17dd8989a37 100644 --- a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java +++ b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java @@ -295,6 +295,8 @@ class PackageManagerShellCommand extends ShellCommand { return runRollbackApp(); case "get-moduleinfo": return runGetModuleInfo(); + case "log-visibility": + return runLogVisibility(); default: { String nextArg = getNextArg(); if (nextArg == null) { @@ -360,6 +362,36 @@ class PackageManagerShellCommand extends ShellCommand { return 1; } + private int runLogVisibility() { + final PrintWriter pw = getOutPrintWriter(); + boolean enable = true; + + String opt; + while ((opt = getNextOption()) != null) { + switch (opt) { + case "--disable": + enable = false; + break; + case "--enable": + enable = true; + break; + default: + pw.println("Error: Unknown option: " + opt); + return -1; + } + } + + String packageName = getNextArg(); + if (packageName != null) { + LocalServices.getService(PackageManagerInternal.class) + .setVisibilityLogging(packageName, enable); + } else { + getErrPrintWriter().println("Error: no package specified"); + return -1; + } + return 1; + } + private int uninstallSystemUpdates() { final PrintWriter pw = getOutPrintWriter(); List failedUninstalls = new LinkedList<>(); @@ -3715,6 +3747,11 @@ class PackageManagerShellCommand extends ShellCommand { pw.println(" --all: show all module info"); pw.println(" --installed: show only installed modules"); pw.println(""); + pw.println(" log-visibility [--enable|--disable] "); + pw.println(" Turns on debug logging when visibility is blocked for the given package."); + pw.println(" --enable: turn on debug logging (default)"); + pw.println(" --disable: turn off debug logging"); + pw.println(""); Intent.printIntentArgsHelp(pw , ""); }