From 80ea048bc62d21588944e72478fc497cd051bf33 Mon Sep 17 00:00:00 2001 From: Harshit Mahajan Date: Mon, 14 Nov 2022 07:33:43 +0000 Subject: [PATCH] Add logic to start FACTORY_RESET only after REBOOT completed In case a package crashes very frequently, the escalation level 5 triggers before the level 4 is finished in RescueParty. Hence the device could get FACTORY_RESET even before we try WARM_REBOOT for rescue. Using the system property PROP_ATTEMPTING_REBOOT ensuring that factory reset level is executed only after reboot has taken place. Bug: b/257269267 Test: atest RescuePartyTest Change-Id: I64dfd0d93d5b17fcfe3cf1cb110e27e926b1052d --- .../java/com/android/server/RescueParty.java | 7 ++++++ .../com/android/server/RescuePartyTest.java | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/services/core/java/com/android/server/RescueParty.java b/services/core/java/com/android/server/RescueParty.java index b56d1fca7f6ac..b4ab254b06e7e 100644 --- a/services/core/java/com/android/server/RescueParty.java +++ b/services/core/java/com/android/server/RescueParty.java @@ -447,6 +447,13 @@ public class RescueParty { thread.start(); break; case LEVEL_FACTORY_RESET: + // Before the completion of Reboot, if any crash happens then PackageWatchdog + // escalates to next level i.e. factory reset, as they happen in separate threads. + // Adding a check to prevent factory reset to execute before above reboot completes. + // Note: this reboot property is not persistent resets after reboot is completed. + if (isRebootPropertySet()) { + break; + } SystemProperties.set(PROP_ATTEMPTING_FACTORY_RESET, "true"); runnable = new Runnable() { @Override diff --git a/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java b/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java index 51fa8517e45f7..e7c384b481522 100644 --- a/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java @@ -252,6 +252,7 @@ public class RescuePartyTest { noteBoot(4); assertTrue(RescueParty.isRebootPropertySet()); + SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); noteBoot(5); assertTrue(RescueParty.isFactoryResetPropertySet()); } @@ -276,6 +277,7 @@ public class RescuePartyTest { noteAppCrash(4, true); assertTrue(RescueParty.isRebootPropertySet()); + SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); noteAppCrash(5, true); assertTrue(RescueParty.isFactoryResetPropertySet()); } @@ -429,6 +431,27 @@ public class RescuePartyTest { for (int i = 0; i < LEVEL_FACTORY_RESET; i++) { noteBoot(i + 1); } + assertFalse(RescueParty.isFactoryResetPropertySet()); + SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + noteBoot(LEVEL_FACTORY_RESET + 1); + assertTrue(RescueParty.isAttemptingFactoryReset()); + assertTrue(RescueParty.isFactoryResetPropertySet()); + } + + @Test + public void testIsAttemptingFactoryResetOnlyAfterRebootCompleted() { + for (int i = 0; i < LEVEL_FACTORY_RESET; i++) { + noteBoot(i + 1); + } + int mitigationCount = LEVEL_FACTORY_RESET + 1; + assertFalse(RescueParty.isFactoryResetPropertySet()); + noteBoot(mitigationCount++); + assertFalse(RescueParty.isFactoryResetPropertySet()); + noteBoot(mitigationCount++); + assertFalse(RescueParty.isFactoryResetPropertySet()); + noteBoot(mitigationCount++); + SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + noteBoot(mitigationCount + 1); assertTrue(RescueParty.isAttemptingFactoryReset()); assertTrue(RescueParty.isFactoryResetPropertySet()); }