From 44dfb6e74e6d6989262b079a64bb0f51a3bdb733 Mon Sep 17 00:00:00 2001 From: Gavin Corkery Date: Tue, 15 Dec 2020 10:53:10 +0000 Subject: [PATCH] Add reboot step to Rescue Party Before showing a factory reset prompt in Rescue Party, first perform a reboot. This will lead to a better UX if the reboot mitigates the issue, rather than showing a factory reset prompt right away. This implementation is resilient against fs-checkpointing by storing the mitigation count in /metadata whenever the mitigation count is incremented. This file will be deleted after reading, so that the mitigation count will be reset after 2 reboots. Added new property to track if Rescue Party is trying to reboot, and extended isAttemptingFactoryReset to handle this property. Without this method returning true, the device will not reboot since PowerManager uses this method to give Rescue Party an exception to reboot early in the boot cycle. This change will slightly break the functionality of debug.crash_sysui and debug.crash_system, since those properties will be cleared by the reboot and the device will not continue crashing after reboot. This will be fixed in a future CL. Test: atest RescuePartyTest Test: setprop debug.crash_system, adb shell stop, adb shell start Bug: 171951174 Change-Id: Ied4d86b9f7608e273c2b396a201fdc8029878a28 --- .../com/android/server/PackageWatchdog.java | 32 ++++++++++++ .../java/com/android/server/RescueParty.java | 51 ++++++++++++++++--- .../com/android/server/RescuePartyTest.java | 24 +++++++-- 3 files changed, 95 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java index 99a1d86d244e4..8b506bac4a851 100644 --- a/services/core/java/com/android/server/PackageWatchdog.java +++ b/services/core/java/com/android/server/PackageWatchdog.java @@ -54,9 +54,13 @@ import libcore.io.IoUtils; import org.xmlpull.v1.XmlPullParserException; +import java.io.BufferedReader; +import java.io.BufferedWriter; import java.io.File; import java.io.FileNotFoundException; import java.io.FileOutputStream; +import java.io.FileReader; +import java.io.FileWriter; import java.io.IOException; import java.io.InputStream; import java.lang.annotation.Retention; @@ -149,6 +153,11 @@ public class PackageWatchdog { private static final String ATTR_PASSED_HEALTH_CHECK = "passed-health-check"; private static final String ATTR_MITIGATION_CALLS = "mitigation-calls"; + // A file containing information about the current mitigation count in the case of a boot loop. + // This allows boot loop information to persist in the case of an fs-checkpoint being + // aborted. + private static final String METADATA_FILE = "/metadata/watchdog/mitigation_count.txt"; + @GuardedBy("PackageWatchdog.class") private static PackageWatchdog sPackageWatchdog; @@ -492,6 +501,7 @@ public class PackageWatchdog { } if (currentObserverToNotify != null) { mBootThreshold.setMitigationCount(mitigationCount); + mBootThreshold.saveMitigationCountToMetadata(); currentObserverToNotify.executeBootLoopMitigation(mitigationCount); } } @@ -1700,9 +1710,31 @@ public class PackageWatchdog { SystemProperties.set(property, Long.toString(newStart)); } + public void saveMitigationCountToMetadata() { + try (BufferedWriter writer = new BufferedWriter(new FileWriter(METADATA_FILE))) { + writer.write(String.valueOf(getMitigationCount())); + } catch (Exception e) { + Slog.e(TAG, "Could not save metadata to file: " + e); + } + } + + public void readMitigationCountFromMetadataIfNecessary() { + File bootPropsFile = new File(METADATA_FILE); + if (bootPropsFile.exists()) { + try (BufferedReader reader = new BufferedReader(new FileReader(METADATA_FILE))) { + String mitigationCount = reader.readLine(); + setMitigationCount(Integer.parseInt(mitigationCount)); + bootPropsFile.delete(); + } catch (Exception e) { + Slog.i(TAG, "Could not read metadata file: " + e); + } + } + } + /** Increments the boot counter, and returns whether the device is bootlooping. */ public boolean incrementAndTest() { + readMitigationCountFromMetadataIfNecessary(); final long now = mSystemClock.uptimeMillis(); if (now - getStart() < 0) { Slog.e(TAG, "Window was less than zero. Resetting start to current time."); diff --git a/services/core/java/com/android/server/RescueParty.java b/services/core/java/com/android/server/RescueParty.java index a1cf8162f0e96..db36e62e44a30 100644 --- a/services/core/java/com/android/server/RescueParty.java +++ b/services/core/java/com/android/server/RescueParty.java @@ -31,6 +31,7 @@ import android.os.Build; import android.os.Bundle; import android.os.Environment; import android.os.FileUtils; +import android.os.PowerManager; import android.os.RecoverySystem; import android.os.RemoteCallback; import android.os.SystemClock; @@ -77,6 +78,7 @@ public class RescueParty { @VisibleForTesting static final String PROP_ENABLE_RESCUE = "persist.sys.enable_rescue"; static final String PROP_ATTEMPTING_FACTORY_RESET = "sys.attempting_factory_reset"; + static final String PROP_ATTEMPTING_REBOOT = "sys.attempting_reboot"; static final String PROP_MAX_RESCUE_LEVEL_ATTEMPTED = "sys.max_rescue_level_attempted"; @VisibleForTesting static final int LEVEL_NONE = 0; @@ -87,7 +89,9 @@ public class RescueParty { @VisibleForTesting static final int LEVEL_RESET_SETTINGS_TRUSTED_DEFAULTS = 3; @VisibleForTesting - static final int LEVEL_FACTORY_RESET = 4; + static final int LEVEL_WARM_REBOOT = 4; + @VisibleForTesting + static final int LEVEL_FACTORY_RESET = 5; @VisibleForTesting static final String PROP_RESCUE_BOOT_COUNT = "sys.rescue_boot_count"; @VisibleForTesting @@ -159,12 +163,24 @@ public class RescueParty { } /** - * Check if we're currently attempting to reboot for a factory reset. + * Check if we're currently attempting to reboot for a factory reset. This method must + * return true if RescueParty tries to reboot early during a boot loop, since the device + * will not be fully booted at this time. + * + * TODO(gavincorkery): Rename method since its scope has expanded. */ public static boolean isAttemptingFactoryReset() { + return isFactoryResetPropertySet() || isRebootPropertySet(); + } + + static boolean isFactoryResetPropertySet() { return SystemProperties.getBoolean(PROP_ATTEMPTING_FACTORY_RESET, false); } + static boolean isRebootPropertySet() { + return SystemProperties.getBoolean(PROP_ATTEMPTING_REBOOT, false); + } + /** * Called when {@code SettingsProvider} has been published, which is a good * opportunity to reset any settings depending on our rescue level. @@ -329,8 +345,10 @@ public class RescueParty { return LEVEL_RESET_SETTINGS_UNTRUSTED_CHANGES; } else if (mitigationCount == 3) { return LEVEL_RESET_SETTINGS_TRUSTED_DEFAULTS; - } else if (mitigationCount >= 4) { - return getMaxRescueLevel(); + } else if (mitigationCount == 4) { + return Math.min(getMaxRescueLevel(), LEVEL_WARM_REBOOT); + } else if (mitigationCount >= 5) { + return Math.min(getMaxRescueLevel(), LEVEL_FACTORY_RESET); } else { Slog.w(TAG, "Expected positive mitigation count, was " + mitigationCount); return LEVEL_NONE; @@ -356,6 +374,8 @@ public class RescueParty { // Try our best to reset all settings possible, and once finished // rethrow any exception that we encountered Exception res = null; + Runnable runnable; + Thread thread; switch (level) { case LEVEL_RESET_SETTINGS_UNTRUSTED_DEFAULTS: try { @@ -396,11 +416,26 @@ public class RescueParty { res = e; } break; - case LEVEL_FACTORY_RESET: + case LEVEL_WARM_REBOOT: // Request the reboot from a separate thread to avoid deadlock on PackageWatchdog // when device shutting down. + SystemProperties.set(PROP_ATTEMPTING_REBOOT, "true"); + runnable = () -> { + try { + PowerManager pm = context.getSystemService(PowerManager.class); + if (pm != null) { + pm.reboot(TAG); + } + } catch (Throwable t) { + logRescueException(level, t); + } + }; + thread = new Thread(runnable); + thread.start(); + break; + case LEVEL_FACTORY_RESET: SystemProperties.set(PROP_ATTEMPTING_FACTORY_RESET, "true"); - Runnable runnable = new Runnable() { + runnable = new Runnable() { @Override public void run() { try { @@ -410,7 +445,7 @@ public class RescueParty { } } }; - Thread thread = new Thread(runnable); + thread = new Thread(runnable); thread.start(); break; } @@ -433,6 +468,7 @@ public class RescueParty { case LEVEL_RESET_SETTINGS_UNTRUSTED_CHANGES: return PackageHealthObserverImpact.USER_IMPACT_LOW; case LEVEL_RESET_SETTINGS_TRUSTED_DEFAULTS: + case LEVEL_WARM_REBOOT: case LEVEL_FACTORY_RESET: return PackageHealthObserverImpact.USER_IMPACT_HIGH; default: @@ -714,6 +750,7 @@ public class RescueParty { case LEVEL_RESET_SETTINGS_UNTRUSTED_DEFAULTS: return "RESET_SETTINGS_UNTRUSTED_DEFAULTS"; case LEVEL_RESET_SETTINGS_UNTRUSTED_CHANGES: return "RESET_SETTINGS_UNTRUSTED_CHANGES"; case LEVEL_RESET_SETTINGS_TRUSTED_DEFAULTS: return "RESET_SETTINGS_TRUSTED_DEFAULTS"; + case LEVEL_WARM_REBOOT: return "WARM_REBOOT"; case LEVEL_FACTORY_RESET: return "FACTORY_RESET"; default: return Integer.toString(level); } diff --git a/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java b/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java index f375421043fd5..fd364ae772400 100644 --- a/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java @@ -233,8 +233,10 @@ public class RescuePartyTest { verifiedTimesMap); noteBoot(4); + assertTrue(RescueParty.isRebootPropertySet()); - assertTrue(RescueParty.isAttemptingFactoryReset()); + noteBoot(5); + assertTrue(RescueParty.isFactoryResetPropertySet()); } @Test @@ -255,7 +257,10 @@ public class RescuePartyTest { /*configResetVerifiedTimesMap=*/ null); notePersistentAppCrash(4); - assertTrue(RescueParty.isAttemptingFactoryReset()); + assertTrue(RescueParty.isRebootPropertySet()); + + notePersistentAppCrash(5); + assertTrue(RescueParty.isFactoryResetPropertySet()); } @Test @@ -306,7 +311,11 @@ public class RescuePartyTest { observer.execute(new VersionedPackage( CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 4); - assertTrue(RescueParty.isAttemptingFactoryReset()); + assertTrue(RescueParty.isRebootPropertySet()); + + observer.execute(new VersionedPackage( + CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 5); + assertTrue(RescueParty.isFactoryResetPropertySet()); } @Test @@ -367,7 +376,11 @@ public class RescuePartyTest { observer.execute(new VersionedPackage( CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 4); - assertTrue(RescueParty.isAttemptingFactoryReset()); + assertTrue(RescueParty.isRebootPropertySet()); + + observer.execute(new VersionedPackage( + CALLING_PACKAGE1, 1), PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 5); + assertTrue(RescueParty.isFactoryResetPropertySet()); } @Test @@ -376,6 +389,7 @@ public class RescuePartyTest { noteBoot(i + 1); } assertTrue(RescueParty.isAttemptingFactoryReset()); + assertTrue(RescueParty.isFactoryResetPropertySet()); } @Test @@ -424,7 +438,7 @@ public class RescuePartyTest { for (int i = 0; i < LEVEL_FACTORY_RESET; i++) { noteBoot(i + 1); } - assertFalse(RescueParty.isAttemptingFactoryReset()); + assertFalse(RescueParty.isFactoryResetPropertySet()); // Restore the property value initialized in SetUp() SystemProperties.set(PROP_DISABLE_FACTORY_RESET_FLAG, "");