futility: updater: Ignore image parsing error in --force

The firmware updater has been improved to support flashing outside DUT
(--ccd, --servo) that also implies more people will use it for devices
with corrupted (or empty) firmware. It's pretty confusing for developers
to see "Cannot load system active firmware" while the flashrom can
actually read and write to the SPI firmware.

The solution here is to allow updating on such devices when --force is
specified (which is automatically applied for --servo and --ccd).

BUG=b:148405957
TEST=make runtests
BRANCH=None

Change-Id: I19e63e3464616bc508639cbfad0d1cf8e99507b0
Signed-off-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2059621
Reviewed-by: Joel Kitching <kitching@chromium.org>

3 files changed, 70 insertions, 41 deletions

diff --git a/futility/updater.c b/futility/updater.c
index f364ff4d..13c712ce 100644
--- a/futility/updater.c
+++ b/futility/updater.c
@@ -1166,13 +1166,14 @@ enum updater_error_codes update_firmware(struct updater_config *cfg)
 		return UPDATE_ERR_PLATFORM;
 
 	if (!image_from->data) {
-		/*
-		 * TODO(hungte) Read only RO_SECTION, VBLOCK_A, VBLOCK_B,
-		 * RO_VPD, RW_VPD, RW_NVRAM, RW_LEGACY.
-		 */
+		int ret;
 		INFO("Loading current system firmware...\n");
-		if (load_system_firmware(image_from, &cfg->tempfiles,
-					 cfg->verbosity) != 0)
+		ret = load_system_firmware(image_from, &cfg->tempfiles,
+					   cfg->verbosity);
+		if (ret == IMAGE_PARSE_FAILURE && cfg->force_update) {
+			WARN("No compatible firmware in system.\n");
+			cfg->check_platform = 0;
+		} else if (ret)
 			return UPDATE_ERR_SYSTEM_IMAGE;
 	}
 	STATUS("Current system: %s (RO:%s, RW/A:%s, RW/B:%s).\n",
diff --git a/futility/updater_utils.c b/futility/updater_utils.c
index 4a91a05f..fadfd04b 100644
--- a/futility/updater_utils.c
+++ b/futility/updater_utils.c
@@ -130,84 +130,105 @@ const char *cbfs_extract_file(const char *image_file,
 /*
  * Loads the firmware information from an FMAP section in loaded firmware image.
  * The section should only contain ASCIIZ string as firmware version.
- * If successful, the return value is zero and *version points to a newly
- * allocated string as firmware version (caller must free it); otherwise
- * failure.
+ * Returns 0 if a non-empty version string is stored in *version, otherwise -1.
  */
 static int load_firmware_version(struct firmware_image *image,
 				 const char *section_name,
 				 char **version)
 {
 	struct firmware_section fwid;
-	find_firmware_section(&fwid, image, section_name);
-	if (fwid.size) {
-		*version = strndup((const char*)fwid.data, fwid.size);
-		/*
-		 * For 'system current' images, the version string may contain
-		 * invalid characters that we do want to strip.
-		 */
-		strip_string(*version, "\xff");
-		return 0;
+	int len = 0;
+
+	/*
+	 * section_name is NULL when parsing the RW versions on a non-vboot
+	 * image (and already warned in load_firmware_image). We still need to
+	 * initialize *version with empty string.
+	 */
+	if (section_name) {
+		find_firmware_section(&fwid, image, section_name);
+		if (fwid.size)
+			len = fwid.size;
+		else
+			WARN("No valid section '%s', missing version info.\n",
+			     section_name);
+	}
+
+	if (!len) {
+		*version = strdup("");
+		return -1;
 	}
-	*version = strdup("");
-	return -1;
+
+	/*
+	 * For 'system current' images, the version string may contain
+	 * invalid characters that we do want to strip.
+	 */
+	*version = strndup((const char *)fwid.data, len);
+	strip_string(*version, "\xff");
+	return 0;
 }
 
 /*
  * Loads a firmware image from file.
  * If archive is provided and file_name is a relative path, read the file from
  * archive.
- * Returns 0 on success, otherwise failure.
+ * Returns IMAGE_LOAD_SUCCESS on success, IMAGE_READ_FAILURE on file I/O
+ * failure, or IMAGE_PARSE_FAILURE for non-vboot images.
  */
 int load_firmware_image(struct firmware_image *image, const char *file_name,
 			struct archive *archive)
 {
+	int ret = IMAGE_LOAD_SUCCESS;
+	const char *section_a = NULL, *section_b = NULL;
+
 	if (!file_name) {
 		ERROR("No file name given\n");
-		return -1;
+		return IMAGE_READ_FAILURE;
 	}
 
 	VB2_DEBUG("Load image file from %s...\n", file_name);
 
 	if (!archive_has_entry(archive, file_name)) {
 		ERROR("Does not exist: %s\n", file_name);
-		return -1;
+		return IMAGE_READ_FAILURE;
 	}
 	if (archive_read_file(archive, file_name, &image->data, &image->size,
 			      NULL) != VB2_SUCCESS) {
 		ERROR("Failed to load %s\n", file_name);
-		return -1;
+		return IMAGE_READ_FAILURE;
 	}
 
 	VB2_DEBUG("Image size: %d\n", image->size);
 	assert(image->data);
 	image->file_name = strdup(file_name);
-
 	image->fmap_header = fmap_find(image->data, image->size);
+
 	if (!image->fmap_header) {
 		ERROR("Invalid image file (missing FMAP): %s\n", file_name);
-		return -1;
+		ret = IMAGE_PARSE_FAILURE;
 	}
 
-	if (!firmware_section_exists(image, FMAP_RO_FRID)) {
-		ERROR("Does not look like VBoot firmware image: %s\n",
-		      file_name);
-		return -1;
-	}
+	if (load_firmware_version(image, FMAP_RO_FRID, &image->ro_version))
+		ret = IMAGE_PARSE_FAILURE;
 
-	load_firmware_version(image, FMAP_RO_FRID, &image->ro_version);
 	if (firmware_section_exists(image, FMAP_RW_FWID_A)) {
-		char **a = &image->rw_version_a, **b = &image->rw_version_b;
-		load_firmware_version(image, FMAP_RW_FWID_A, a);
-		load_firmware_version(image, FMAP_RW_FWID_B, b);
+		section_a = FMAP_RW_FWID_A;
+		section_b = FMAP_RW_FWID_B;
 	} else if (firmware_section_exists(image, FMAP_RW_FWID)) {
-		char **a = &image->rw_version_a, **b = &image->rw_version_b;
-		load_firmware_version(image, FMAP_RW_FWID, a);
-		load_firmware_version(image, FMAP_RW_FWID, b);
-	} else {
+		section_a = FMAP_RW_FWID;
+		section_b = FMAP_RW_FWID;
+	} else if (!ret) {
 		ERROR("Unsupported VBoot firmware (no RW ID): %s\n", file_name);
+		ret = IMAGE_PARSE_FAILURE;
 	}
-	return 0;
+
+	/*
+	 * Load and initialize both RW A and B sections.
+	 * Note some unit tests will create only RW A.
+	 */
+	load_firmware_version(image, section_a, &image->rw_version_a);
+	load_firmware_version(image, section_b, &image->rw_version_b);
+
+	return ret;
 }
 
 /*
diff --git a/futility/updater_utils.h b/futility/updater_utils.h
index 5e6e99cd..622e4e2e 100644
--- a/futility/updater_utils.h
+++ b/futility/updater_utils.h
@@ -64,11 +64,18 @@ struct firmware_image {
 	FmapHeader *fmap_header;
 };
 
+enum {
+	IMAGE_LOAD_SUCCESS = 0,
+	IMAGE_READ_FAILURE = -1,
+	IMAGE_PARSE_FAILURE = -2,
+};
+
 /*
  * Loads a firmware image from file.
  * If archive is provided and file_name is a relative path, read the file from
  * archive.
- * Returns 0 on success, otherwise failure.
+ * Returns IMAGE_LOAD_SUCCESS on success, IMAGE_READ_FAILURE on file I/O
+ * failure, or IMAGE_PARSE_FAILURE for non-vboot images.
  */
 int load_firmware_image(struct firmware_image *image, const char *file_name,
 			struct archive *archive);