Revert "vboot_reference/futility: Port W path to using libflashrom"stabilize-14388.62.Bstabilize-14388.61.Bstabilize-14388.52.Brelease-R98-14388.B

This reverts commit 483dff64346fd30224df5b66bf76124aeea7bb12.

Changing the write path in futility to use libflashrom had the side
effect of skipping the code in cli_classic that created a powerd lock
file. This could result in powerd suspending during a firmware update,
corrupting the RW firmware.

Revert back to subprocessing flashrom for M98.

BUG=b:214485250
BRANCH=release-R98-14388.B
TEST=emerge-grunt vboot_reference
TEST=flashed grunt DUT with R98-14388.30.0 test image, deployed \
     vboot_reference with reverts
TEST=ran `futility --force -i image.bin`, monitored processes \
     with `ps -ef | grep flashrom` verified that flashrom ran as \
     a separate process for read/wp-status/write ops
TEST=verified /run/lock/power_override/flashrom.lock was created \
     during update
TEST=ran `futility --force -i image.bin --wp=0`, verified only RW \
     sections A/B were written
TEST=ran `futility --force -i image.bin`, waited for write to start, \
     sent restart msg with: `dbus-send --type=method_call --system \
     --dest=org.chromium.PowerManager /org/chromium/PowerManager \
     org.chromium.PowerManager.RequestRestart`, verified restart was \
     deferred until firmware write finished

Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Change-Id: I81ba6e45aa926d7ff930d280d2f4a5ef31c59fa2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3388976
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>

2 files changed, 30 insertions, 121 deletions

diff --git a/Makefile b/Makefile
index 97e469cc..6a6dca7d 100644
--- a/Makefile
+++ b/Makefile
@@ -284,8 +284,6 @@ ifneq ($(filter-out 0,${HAVE_LIBZIP}),)
   LIBZIP_LIBS := $(shell ${PKG_CONFIG} --libs libzip)
 endif
 
-FLASHROM_LIBS := $(shell ${PKG_CONFIG} --libs flashrom)
-
 # Determine QEMU architecture needed, if any
 ifeq (${ARCH},${HOST_ARCH})
   # Same architecture; no need for QEMU
@@ -1029,7 +1027,7 @@ signing_install: $(if ${SDK_BUILD},\
 futil: ${FUTIL_BIN}
 
 # FUTIL_LIBS is shared by FUTIL_BIN and TEST_FUTIL_BINS.
-FUTIL_LIBS = ${CRYPTO_LIBS} ${LIBZIP_LIBS} ${FLASHROM_LIBS}
+FUTIL_LIBS = ${CRYPTO_LIBS} ${LIBZIP_LIBS}
 
 ${FUTIL_BIN}: LDLIBS += ${FUTIL_LIBS}
 ${FUTIL_BIN}: ${FUTIL_OBJS} ${UTILLIB} ${FWLIB}
diff --git a/futility/updater_utils.c b/futility/updater_utils.c
index 039788a2..51936a52 100644
--- a/futility/updater_utils.c
+++ b/futility/updater_utils.c
@@ -9,14 +9,11 @@
 #include <limits.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <string.h>
 #include <unistd.h>
 #if defined (__FreeBSD__) || defined(__OpenBSD__)
 #include <sys/wait.h>
 #endif
 
-#include <libflashrom.h>
-
 #include "2common.h"
 #include "crossystem.h"
 #include "host_misc.h"
@@ -28,6 +25,7 @@
 
 enum flashrom_ops {
 	FLASHROM_READ,
+	FLASHROM_WRITE,
 	FLASHROM_WP_STATUS,
 };
 
@@ -567,6 +565,11 @@ static int host_flashrom(enum flashrom_ops op, const char *image_path,
 		assert(image_path);
 		break;
 
+	case FLASHROM_WRITE:
+		op_cmd = "-w";
+		assert(image_path);
+		break;
+
 	case FLASHROM_WP_STATUS:
 		op_cmd = "--wp-status";
 		assert(image_path == NULL);
@@ -584,7 +587,7 @@ static int host_flashrom(enum flashrom_ops op, const char *image_path,
 	if (!extra)
 		extra = "";
 
-	/* TODO(b/203715651): link with flashrom directly. */
+	/* TODO(hungte) In future we should link with flashrom directly. */
 	ASPRINTF(&command, "flashrom %s %s -p %s %s %s %s %s", op_cmd,
 		 image_path, programmer, dash_i, section_name, extra,
 		 postfix);
@@ -615,119 +618,6 @@ static int host_flashrom(enum flashrom_ops op, const char *image_path,
 	return r;
 }
 
-static int flashrom_print_cb(enum flashrom_log_level level, const char *fmt,
-			     va_list ap)
-{
-	int ret = 0;
-	enum flashrom_log_level verbose_screen = FLASHROM_MSG_INFO;
-	FILE *output_type = (level < verbose_screen) ? stderr : stdout;
-
-	if (level > verbose_screen)
-		return ret;
-
-#define COLOUR_RESET "\033[0;m"
-#define MAGENTA_TEXT "\033[35;1m"
-
-	if (level != FLASHROM_MSG_SPEW)
-		fprintf(output_type, MAGENTA_TEXT);
-
-	ret = vfprintf(output_type, fmt, ap);
-	/* msg_*spew often happens inside chip accessors
-	 * in possibly time-critical operations.
-	 * Don't slow them down by flushing.
-	 */
-	if (level != FLASHROM_MSG_SPEW) {
-		fprintf(output_type, COLOUR_RESET);
-		fflush(output_type);
-	}
-
-	return ret;
-}
-
-static char *flashrom_extract_params(const char *str, char **prog, char **params)
-{
-	char *tmp = strdup(str);
-	*prog = strtok(tmp, ":");
-	*params = strtok(NULL, "");
-	return tmp;
-}
-
-static int host_flashrom_write(const struct firmware_image *image,
-			       const char *region,
-			       const struct firmware_image *diff_image)
-{
-	int r = 0;
-	size_t len = 0;
-
-	char *programmer, *params;
-	char *tmp = flashrom_extract_params(image->programmer, &programmer, &params);
-
-	struct flashrom_programmer *prog = NULL;
-	struct flashrom_flashctx *flashctx = NULL;
-	struct flashrom_layout *layout = NULL;
-
-	flashrom_set_log_callback((flashrom_log_callback *)&flashrom_print_cb);
-
-	r |= flashrom_init(1);
-	r |= flashrom_programmer_init(&prog, programmer, params);
-	r |= flashrom_flash_probe(&flashctx, prog, NULL);
-
-	len = flashrom_flash_getsize(flashctx);
-	if (len == 0) {
-		ERROR("zero sized flash detected\n");
-		r = -1;
-		goto err_cleanup;
-	}
-
-	if (diff_image) {
-		if (diff_image->size != image->size) {
-			ERROR("diff_image->size != image->size");
-			r = -1;
-			goto err_cleanup;
-		}
-	}
-
-	if (region) {
-		r = flashrom_layout_read_fmap_from_buffer(
-			&layout, flashctx, (const uint8_t *)image->data,
-			image->size);
-		if (r > 0) {
-			WARN("could not read fmap from image, r=%d, "
-				"falling back to read from rom\n", r);
-			r = flashrom_layout_read_fmap_from_rom(
-				&layout, flashctx, 0, len);
-			if (r > 0) {
-				ERROR("could not read fmap from rom, r=%d\n", r);
-				r = -1;
-				goto err_cleanup;
-			}
-		}
-		// empty region causes seg fault in API.
-		r |= flashrom_layout_include_region(layout, region);
-		if (r > 0) {
-			ERROR("could not include region = '%s'\n", region);
-			r = -1;
-			goto err_cleanup;
-		}
-	}
-
-	flashrom_flag_set(flashctx, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, true);
-	flashrom_flag_set(flashctx, FLASHROM_FLAG_VERIFY_AFTER_WRITE, true);
-	if (diff_image) /* equiv --noverify --flash-contents=diff_image at cli */
-		flashrom_flag_set(flashctx, FLASHROM_FLAG_VERIFY_AFTER_WRITE, false);
-
-	r |= flashrom_image_write(flashctx, image->data, image->size,
-				  diff_image ? diff_image->data : NULL);
-
-err_cleanup:
-	r |= flashrom_programmer_shutdown(prog);
-	flashrom_layout_release(layout);
-	flashrom_flash_release(flashctx);
-	free(tmp);
-
-	return r;
-}
-
 /* Helper function to return write protection status via given programmer. */
 enum wp_state host_get_wp(const char *programmer)
 {
@@ -783,7 +673,28 @@ int write_system_firmware(const struct firmware_image *image,
 			  struct tempfile *tempfiles,
 			  int verbosity)
 {
-	return host_flashrom_write(image, section_name, diff_image);
+	const char *tmp_path = get_firmware_image_temp_file(image, tempfiles);
+	const char *tmp_diff = NULL;
+
+	const char *programmer = image->programmer;
+	char *extra = NULL;
+	int r;
+
+	if (!tmp_path)
+		return -1;
+
+	if (diff_image) {
+		tmp_diff = get_firmware_image_temp_file(
+				diff_image, tempfiles);
+		if (!tmp_diff)
+			return -1;
+		ASPRINTF(&extra, "--noverify --flash-contents=%s", tmp_diff);
+	}
+
+	r = host_flashrom(FLASHROM_WRITE, tmp_path, programmer, verbosity,
+			  section_name, extra);
+	free(extra);
+	return r;
 }
 
 /* Helper function to configure all properties. */