futility: updater: Only apply the preserve_me quirk for autoupdates.

The preserve_me applies for all non-factory updates for firmware with
the quirk enabled. It was only really intended to apply to firmware
updates during autoupdates, that is --mode=autoupdate. Instead, we
checked for an archive, which is always set, possibly a fallback
directory archive rather than an archive file, resulting in it being
used except for --mode=factory. Switch the condition to TRY_UPDATE_AUTO
instead so only --mode=autoupdate enables the preserve_me quirk.

BUG=b:255447297
TEST=futility update -i /tmp/image.bin doesn't apply the quirk
     futility update -i /tmp/image.bin -m autoupdate applies the quirk
BRANCH=None

Change-Id: I7459f027a918dc70cbde1bfc6f5da2b549bcc513
Signed-off-by: Sam McNally <sammc@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/4050014
Reviewed-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>

4 files changed, 32 insertions, 21 deletions

diff --git a/futility/updater.c b/futility/updater.c
index 51418847..6129b73c 100644
--- a/futility/updater.c
+++ b/futility/updater.c
@@ -21,13 +21,6 @@
 static const char ROOTKEY_HASH_DEV[] =
 		"b11d74edd286c144e1135b49e7f0bc20cf041f10";
 
-enum try_update_type {
-	TRY_UPDATE_OFF = 0,
-	TRY_UPDATE_AUTO,
-	TRY_UPDATE_DEFERRED_HOLD,
-	TRY_UPDATE_DEFERRED_APPLY,
-};
-
 enum target_type {
 	TARGET_SELF,
 	TARGET_UPDATE,
diff --git a/futility/updater.h b/futility/updater.h
index 08564b58..3f77c332 100644
--- a/futility/updater.h
+++ b/futility/updater.h
@@ -61,6 +61,13 @@ enum {
 	EC_RECOVERY_DONE
 };
 
+enum try_update_type {
+	TRY_UPDATE_OFF = 0,
+	TRY_UPDATE_AUTO,
+	TRY_UPDATE_DEFERRED_HOLD,
+	TRY_UPDATE_DEFERRED_APPLY,
+};
+
 struct updater_config {
 	struct firmware_image image, image_current;
 	struct firmware_image ec_image, pd_image;
@@ -68,7 +75,7 @@ struct updater_config {
 	struct quirk_entry quirks[QUIRK_MAX];
 	struct u_archive *archive;
 	struct tempfile tempfiles;
-	int try_update;
+	enum try_update_type try_update;
 	int force_update;
 	int legacy_update;
 	int factory_update;
diff --git a/futility/updater_quirks.c b/futility/updater_quirks.c
index 73b3b897..dab86ec8 100644
--- a/futility/updater_quirks.c
+++ b/futility/updater_quirks.c
@@ -391,7 +391,7 @@ static int quirk_ec_partial_recovery(struct updater_config *cfg)
  *
  * Updating ME region while SoC is in S0 state is an unsupported use-case. On
  * recent platforms, we are seeing issues more frequently because of this use-
- * case. For the firmware updates performed using firmware update archive,
+ * case. For the firmware updates performed for autoupdate firmware updates,
  * preserve the ME region so that it gets updated in the successive boot.
  *
  * Returns:
@@ -400,21 +400,17 @@ static int quirk_ec_partial_recovery(struct updater_config *cfg)
  */
 static int quirk_preserve_me(struct updater_config *cfg)
 {
-	/* For a factory update donot preserve ME. */
-	if (cfg->factory_update) {
-	       WARN("Factory update. Not preserving ME.\n");
-	       return 0;
-	}
-
 	/*
-	 * For a non-archive update, the post boot script that updates ME
-	 * does not have access to the firmware image after the boot. Hence
-	 * donot preserve ME.
+	 * Only preserve the ME if performing an autoupdate-mode firmware
+	 * update. Recovery, factory and any other update modes cannot leave the
+	 * ME as is. Otherwise, a recovery firmware update cannot be relied upon
+	 * to update the ME to a valid version for WP-disabled devices.
 	 */
-	if (!cfg->archive) {
-		WARN("Update using a non-archive image. Not preserving ME.\n");
+	if (cfg->try_update == TRY_UPDATE_OFF) {
+		INFO("No auto-update requested. Not preserving ME.\n");
 		return 0;
 	}
+	INFO("Auto-update requested. Preserving ME.\n");
 
 	/*
 	 * b/213706510: subratabanik@ confirmed CSE may modify itself while we
diff --git a/tests/futility/test_update.sh b/tests/futility/test_update.sh
index f2ca3034..41dd3608 100755
--- a/tests/futility/test_update.sh
+++ b/tests/futility/test_update.sh
@@ -378,10 +378,25 @@ test_update "Full update (--quirks preserve_me with non-host programmer)" \
 	-p raiden_debug_spi:target=AP
 
 test_update "Full update (--quirks preserve_me)" \
-	"${FROM_IMAGE}" "${TMP}.expected.me_preserved" \
+	"${FROM_IMAGE}" "${TMP}.expected.full" \
 	--quirks preserve_me \
 	-i "${TO_IMAGE}" --wp=0 --sys_props 0,0x10001,1
 
+test_update "Full update (--quirks preserve_me, autoupdate)" \
+	"${FROM_IMAGE}" "${TMP}.expected.me_preserved" \
+	--quirks preserve_me -m autoupdate \
+	-i "${TO_IMAGE}" --wp=0 --sys_props 0,0x10001,1
+
+test_update "Full update (--quirks preserve_me, deferupdate_hold)" \
+	"${FROM_IMAGE}" "${TMP}.expected.me_preserved" \
+	--quirks preserve_me -m deferupdate_hold \
+	-i "${TO_IMAGE}" --wp=0 --sys_props 0,0x10001,1
+
+test_update "Full update (--quirks preserve_me, factory)" \
+	"${FROM_IMAGE}" "${TMP}.expected.full" \
+	--quirks preserve_me -m factory \
+	-i "${TO_IMAGE}" --wp=0 --sys_props 0,0x10001,1
+
 # Test archive and manifest. CL_TAG is for custom_label_tag.
 A="${TMP}.archive"
 mkdir -p "${A}/bin"