From c8f1e2b38c176613d9d2388cb52d9c0b10fc4abd Mon Sep 17 00:00:00 2001 From: Hongyi Zhang Date: Mon, 9 Nov 2020 10:16:07 -0800 Subject: [PATCH] Constrain Non-DeviceConfig settings reset times & unify DeviceConfig reset mode 1. For Non-DeviceConfig settings(Settings.Secure and Settings.Global), constrain that it'll be reset only once(with the corresponding ResetMode) per rescue level even if the level is executed multiple times from multiple packages. Per-package rescue level is only meaningful for Settings.Config where package<->namespace access is recorded. Thus restoring Non-DeviceConfig settings to their pre-S resetting sequence(e.g. no repeated or "de-escalated" reset mode) to avoid unexpected regressions. 2. Simplify all DeviceConfig resets to Reset Mode RESET_MODE_TRUSTED_DEFAULTS. This is because there is no "UNTRUSTED" defaults in Settings.Config, while resetting to that step will reset all values to null, vs resetting to "TRUSTED" defaults *deletes* all KVPs. It's the same effect when calling DeviceConfig.getProperies, but the difference causes unnecessary complications when banning namespaces(e.g. "null map"'s banning hash is different from empty map). Next steps: 1. Rename the four rescue levels away from Reset modes since that's not how Settings.Config resets per level(even before this change). 2. Move bootloop mitigation to also infer rescue level from the "attempted times" provided by PackageWatchdog. Bug: 162425385 Test: atest RescueParty Change-Id: Ib49ae9f9b7e0f5e255d038e088d4e7c746dc3339 --- .../java/com/android/server/RescueParty.java | 92 ++++++++++----- .../com/android/server/RescuePartyTest.java | 107 ++++++++++++++++-- 2 files changed, 161 insertions(+), 38 deletions(-) diff --git a/services/core/java/com/android/server/RescueParty.java b/services/core/java/com/android/server/RescueParty.java index 6206f7a583d16..d04949ac54dba 100644 --- a/services/core/java/com/android/server/RescueParty.java +++ b/services/core/java/com/android/server/RescueParty.java @@ -78,6 +78,7 @@ public class RescueParty { @VisibleForTesting static final String PROP_RESCUE_LEVEL = "sys.rescue_level"; static final String PROP_ATTEMPTING_FACTORY_RESET = "sys.attempting_factory_reset"; + static final String PROP_MAX_RESCUE_LEVEL_ATTEMPTED = "sys.max_rescue_level_attempted"; @VisibleForTesting static final int LEVEL_NONE = 0; @VisibleForTesting @@ -94,6 +95,8 @@ public class RescueParty { static final String TAG = "RescueParty"; @VisibleForTesting static final long DEFAULT_OBSERVING_DURATION_MS = TimeUnit.DAYS.toMillis(2); + @VisibleForTesting + static final int DEVICE_CONFIG_RESET_MODE = Settings.RESET_MODE_TRUSTED_DEFAULTS; private static final String NAME = "rescue-party-observer"; @@ -225,7 +228,7 @@ public class RescueParty { if (NAMESPACE_CONFIGURATION.equals(resetNativeCategories[i])) { continue; } - DeviceConfig.resetToDefaults(Settings.RESET_MODE_TRUSTED_DEFAULTS, + DeviceConfig.resetToDefaults(DEVICE_CONFIG_RESET_MODE, resetNativeCategories[i]); } } @@ -300,15 +303,48 @@ public class RescueParty { private static void executeRescueLevelInternal(Context context, int level, @Nullable String failedPackage) throws Exception { FrameworkStatsLog.write(FrameworkStatsLog.RESCUE_PARTY_RESET_REPORTED, level); + // Try our best to reset all settings possible, and once finished + // rethrow any exception that we encountered + Exception res = null; switch (level) { case LEVEL_RESET_SETTINGS_UNTRUSTED_DEFAULTS: - resetAllSettings(context, Settings.RESET_MODE_UNTRUSTED_DEFAULTS, failedPackage); + try { + resetAllSettingsIfNecessary(context, Settings.RESET_MODE_UNTRUSTED_DEFAULTS, + level); + } catch (Exception e) { + res = e; + } + try { + resetDeviceConfig(context, /*isScoped=*/true, failedPackage); + } catch (Exception e) { + res = e; + } break; case LEVEL_RESET_SETTINGS_UNTRUSTED_CHANGES: - resetAllSettings(context, Settings.RESET_MODE_UNTRUSTED_CHANGES, failedPackage); + try { + resetAllSettingsIfNecessary(context, Settings.RESET_MODE_UNTRUSTED_CHANGES, + level); + } catch (Exception e) { + res = e; + } + try { + resetDeviceConfig(context, /*isScoped=*/true, failedPackage); + } catch (Exception e) { + res = e; + } break; case LEVEL_RESET_SETTINGS_TRUSTED_DEFAULTS: - resetAllSettings(context, Settings.RESET_MODE_TRUSTED_DEFAULTS, failedPackage); + try { + resetAllSettingsIfNecessary(context, Settings.RESET_MODE_TRUSTED_DEFAULTS, + level); + } catch (Exception e) { + res = e; + } + try { + resetDeviceConfig(context, /*isScoped=*/false, failedPackage); + } catch (Exception e) { + res = e; + } break; case LEVEL_FACTORY_RESET: // Request the reboot from a separate thread to avoid deadlock on PackageWatchdog @@ -328,6 +364,10 @@ public class RescueParty { thread.start(); break; } + + if (res != null) { + throw res; + } } private static void logRescueException(int level, Throwable t) { @@ -350,17 +390,17 @@ public class RescueParty { } } - private static void resetAllSettings(Context context, int mode, @Nullable String failedPackage) - throws Exception { + private static void resetAllSettingsIfNecessary(Context context, int mode, + int level) throws Exception { + // No need to reset Settings again if they are already reset in the current level once. + if (SystemProperties.getInt(PROP_MAX_RESCUE_LEVEL_ATTEMPTED, LEVEL_NONE) >= level) { + return; + } + SystemProperties.set(PROP_MAX_RESCUE_LEVEL_ATTEMPTED, Integer.toString(level)); // Try our best to reset all settings possible, and once finished // rethrow any exception that we encountered Exception res = null; final ContentResolver resolver = context.getContentResolver(); - try { - resetDeviceConfig(context, mode, failedPackage); - } catch (Exception e) { - res = new RuntimeException("Failed to reset config settings", e); - } try { Settings.Global.resetToDefaultsAsUser(resolver, null, mode, UserHandle.USER_SYSTEM); } catch (Exception e) { @@ -378,16 +418,21 @@ public class RescueParty { } } - private static void resetDeviceConfig(Context context, int resetMode, - @Nullable String failedPackage) { - if (!shouldPerformScopedResets(resetMode) || failedPackage == null) { - resetAllAffectedNamespaces(context, resetMode); - } else { - performScopedReset(context, resetMode, failedPackage); + private static void resetDeviceConfig(Context context, boolean isScoped, + @Nullable String failedPackage) throws Exception { + final ContentResolver resolver = context.getContentResolver(); + try { + if (!isScoped || failedPackage == null) { + resetAllAffectedNamespaces(context); + } else { + performScopedReset(context, failedPackage); + } + } catch (Exception e) { + throw new RuntimeException("Failed to reset config settings", e); } } - private static void resetAllAffectedNamespaces(Context context, int resetMode) { + private static void resetAllAffectedNamespaces(Context context) { RescuePartyObserver rescuePartyObserver = RescuePartyObserver.getInstance(context); Set allAffectedNamespaces = rescuePartyObserver.getAllAffectedNamespaceSet(); @@ -401,16 +446,11 @@ public class RescueParty { if (NAMESPACE_CONFIGURATION.equals(namespace)) { continue; } - DeviceConfig.resetToDefaults(resetMode, namespace); + DeviceConfig.resetToDefaults(DEVICE_CONFIG_RESET_MODE, namespace); } } - private static boolean shouldPerformScopedResets(int resetMode) { - return resetMode <= Settings.RESET_MODE_UNTRUSTED_CHANGES; - } - - private static void performScopedReset(Context context, int resetMode, - @NonNull String failedPackage) { + private static void performScopedReset(Context context, @NonNull String failedPackage) { RescuePartyObserver rescuePartyObserver = RescuePartyObserver.getInstance(context); Set affectedNamespaces = rescuePartyObserver.getAffectedNamespaceSet( failedPackage); @@ -428,7 +468,7 @@ public class RescueParty { if (NAMESPACE_CONFIGURATION.equals(namespace)) { continue; } - DeviceConfig.resetToDefaults(resetMode, namespace); + DeviceConfig.resetToDefaults(DEVICE_CONFIG_RESET_MODE, namespace); } } } diff --git a/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java b/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java index da4071b6b9dee..ebd4a4c5378fe 100644 --- a/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java @@ -191,10 +191,12 @@ public class RescuePartyTest { RescueParty.onSettingsProviderPublished(mMockContext); verify(() -> Settings.Config.registerMonitorCallback(eq(mMockContentResolver), mMonitorCallbackCaptor.capture())); + HashMap verifiedTimesMap = new HashMap(); noteBoot(); - verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, /*resetNamespaces=*/ null); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, /*resetNamespaces=*/ null, + verifiedTimesMap); assertEquals(RescueParty.LEVEL_RESET_SETTINGS_UNTRUSTED_DEFAULTS, SystemProperties.getInt(RescueParty.PROP_RESCUE_LEVEL, RescueParty.LEVEL_NONE)); @@ -208,13 +210,15 @@ public class RescuePartyTest { noteBoot(); - verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_CHANGES, expectedAllResetNamespaces); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_CHANGES, expectedAllResetNamespaces, + verifiedTimesMap); assertEquals(RescueParty.LEVEL_RESET_SETTINGS_UNTRUSTED_CHANGES, SystemProperties.getInt(RescueParty.PROP_RESCUE_LEVEL, RescueParty.LEVEL_NONE)); noteBoot(); - verifySettingsResets(Settings.RESET_MODE_TRUSTED_DEFAULTS, expectedAllResetNamespaces); + verifySettingsResets(Settings.RESET_MODE_TRUSTED_DEFAULTS, expectedAllResetNamespaces, + verifiedTimesMap); assertEquals(RescueParty.LEVEL_RESET_SETTINGS_TRUSTED_DEFAULTS, SystemProperties.getInt(RescueParty.PROP_RESCUE_LEVEL, RescueParty.LEVEL_NONE)); @@ -228,15 +232,18 @@ public class RescuePartyTest { public void testPersistentAppCrashDetectionWithExecutionForAllRescueLevels() { notePersistentAppCrash(1); - verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, /*resetNamespaces=*/ null); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, /*resetNamespaces=*/ null, + /*configResetVerifiedTimesMap=*/ null); notePersistentAppCrash(2); - verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_CHANGES, /*resetNamespaces=*/ null); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_CHANGES, /*resetNamespaces=*/ null, + /*configResetVerifiedTimesMap=*/ null); notePersistentAppCrash(3); - verifySettingsResets(Settings.RESET_MODE_TRUSTED_DEFAULTS, /*resetNamespaces=*/ null); + verifySettingsResets(Settings.RESET_MODE_TRUSTED_DEFAULTS, /*resetNamespaces=*/ null, + /*configResetVerifiedTimesMap=*/ null); notePersistentAppCrash(4); assertTrue(RescueParty.isAttemptingFactoryReset()); @@ -272,17 +279,82 @@ public class RescuePartyTest { final String[] expectedResetNamespaces = new String[]{NAMESPACE1, NAMESPACE2}; final String[] expectedAllResetNamespaces = new String[]{NAMESPACE1, NAMESPACE2, NAMESPACE3}; + HashMap verifiedTimesMap = new HashMap(); observer.execute(new VersionedPackage( CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_CRASH, 1); - verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, expectedResetNamespaces); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, expectedResetNamespaces, + verifiedTimesMap); observer.execute(new VersionedPackage( CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 2); - verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_CHANGES, expectedResetNamespaces); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_CHANGES, expectedResetNamespaces, + verifiedTimesMap); observer.execute(new VersionedPackage( CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 3); - verifySettingsResets(Settings.RESET_MODE_TRUSTED_DEFAULTS, expectedAllResetNamespaces); + verifySettingsResets(Settings.RESET_MODE_TRUSTED_DEFAULTS, expectedAllResetNamespaces, + verifiedTimesMap); + + observer.execute(new VersionedPackage( + CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 4); + assertTrue(RescueParty.isAttemptingFactoryReset()); + } + + @Test + public void testNonDeviceConfigSettingsOnlyResetOncePerLevel() { + RescueParty.onSettingsProviderPublished(mMockContext); + verify(() -> Settings.Config.registerMonitorCallback(eq(mMockContentResolver), + mMonitorCallbackCaptor.capture())); + + // Record DeviceConfig accesses + RescuePartyObserver observer = RescuePartyObserver.getInstance(mMockContext); + RemoteCallback monitorCallback = mMonitorCallbackCaptor.getValue(); + monitorCallback.sendResult(getConfigAccessBundle(CALLING_PACKAGE1, NAMESPACE1)); + monitorCallback.sendResult(getConfigAccessBundle(CALLING_PACKAGE1, NAMESPACE2)); + monitorCallback.sendResult(getConfigAccessBundle(CALLING_PACKAGE2, NAMESPACE2)); + monitorCallback.sendResult(getConfigAccessBundle(CALLING_PACKAGE2, NAMESPACE3)); + // Fake DeviceConfig value changes + monitorCallback.sendResult(getConfigNamespaceUpdateBundle(NAMESPACE1)); + monitorCallback.sendResult(getConfigNamespaceUpdateBundle(NAMESPACE2)); + monitorCallback.sendResult(getConfigNamespaceUpdateBundle(NAMESPACE3)); + // Perform and verify scoped resets + final String[] expectedPackage1ResetNamespaces = new String[]{NAMESPACE1, NAMESPACE2}; + final String[] expectedPackage2ResetNamespaces = new String[]{NAMESPACE2, NAMESPACE3}; + final String[] expectedAllResetNamespaces = + new String[]{NAMESPACE1, NAMESPACE2, NAMESPACE3}; + HashMap verifiedTimesMap = new HashMap(); + observer.execute(new VersionedPackage( + CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_CRASH, 1); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, + expectedPackage1ResetNamespaces, verifiedTimesMap); + + // Settings.Global & Settings.Secure should still remain the same execution times. + observer.execute(new VersionedPackage( + CALLING_PACKAGE2, 1), PackageWatchdog.FAILURE_REASON_APP_CRASH, 1); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, + expectedPackage2ResetNamespaces, verifiedTimesMap); + + observer.execute(new VersionedPackage( + CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 2); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_CHANGES, + expectedPackage1ResetNamespaces, verifiedTimesMap); + + // Settings.Global & Settings.Secure should still remain the same execution times. + observer.execute(new VersionedPackage( + CALLING_PACKAGE2, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 2); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_CHANGES, + expectedPackage2ResetNamespaces, verifiedTimesMap); + + observer.execute(new VersionedPackage( + CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 3); + verifySettingsResets(Settings.RESET_MODE_TRUSTED_DEFAULTS, expectedAllResetNamespaces, + verifiedTimesMap); + + // Settings.Global & Settings.Secure should still remain the same execution times. + observer.execute(new VersionedPackage( + CALLING_PACKAGE2, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 3); + verifySettingsResets(Settings.RESET_MODE_TRUSTED_DEFAULTS, expectedAllResetNamespaces, + verifiedTimesMap); observer.execute(new VersionedPackage( CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 4); @@ -303,7 +375,8 @@ public class RescuePartyTest { RescueParty.onSettingsProviderPublished(mMockContext); - verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, /*resetNamespaces=*/ null); + verifySettingsResets(Settings.RESET_MODE_UNTRUSTED_DEFAULTS, /*resetNamespaces=*/ null, + /*configResetVerifiedTimesMap=*/ null); assertEquals(RescueParty.LEVEL_RESET_SETTINGS_UNTRUSTED_DEFAULTS, SystemProperties.getInt(RescueParty.PROP_RESCUE_LEVEL, RescueParty.LEVEL_NONE)); } @@ -415,7 +488,8 @@ public class RescuePartyTest { assertEquals(observer.onBootLoop(), PackageHealthObserverImpact.USER_IMPACT_HIGH); } - private void verifySettingsResets(int resetMode, String[] resetNamespaces) { + private void verifySettingsResets(int resetMode, String[] resetNamespaces, + HashMap configResetVerifiedTimesMap) { verify(() -> Settings.Global.resetToDefaultsAsUser(mMockContentResolver, null, resetMode, UserHandle.USER_SYSTEM)); verify(() -> Settings.Secure.resetToDefaultsAsUser(eq(mMockContentResolver), isNull(), @@ -425,7 +499,16 @@ public class RescuePartyTest { verify(() -> DeviceConfig.resetToDefaults(anyInt(), anyString()), never()); } else { for (String namespace : resetNamespaces) { - verify(() -> DeviceConfig.resetToDefaults(resetMode, namespace)); + int verifiedTimes = 0; + if (configResetVerifiedTimesMap != null + && configResetVerifiedTimesMap.get(namespace) != null) { + verifiedTimes = configResetVerifiedTimesMap.get(namespace); + } + verify(() -> DeviceConfig.resetToDefaults(RescueParty.DEVICE_CONFIG_RESET_MODE, + namespace), times(verifiedTimes + 1)); + if (configResetVerifiedTimesMap != null) { + configResetVerifiedTimesMap.put(namespace, verifiedTimes + 1); + } } } }