From 8d0107d9ba0d7cecebe3da7e7138b0fec9ae6abe Mon Sep 17 00:00:00 2001 From: Nikolai Artemiev Date: Fri, 14 Jan 2022 17:09:42 +1100 Subject: Revert "vboot_reference/futility: Port W path to using libflashrom" 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 Change-Id: I81ba6e45aa926d7ff930d280d2f4a5ef31c59fa2 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3388976 Reviewed-by: Edward O'Callaghan --- Makefile | 4 +- futility/updater_utils.c | 147 ++++++++++------------------------------------- 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 #include #include -#include #include #if defined (__FreeBSD__) || defined(__OpenBSD__) #include #endif -#include - #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, ¶ms); - - 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. */ -- cgit v1.2.1