From 774dcffc369989b70595e27dc96ee191bd48e34f Mon Sep 17 00:00:00 2001 From: Edward O'Callaghan Date: Mon, 6 Jun 2022 20:01:04 +1000 Subject: [PATCH] util/cbfstool: Decouple elogtool from vboot_ref flashrom code Currently elogtool sub-proccesses flashrom as calling libflashrom requires a missing function from the previous flashrom release. Pending a new release of flashrom we must continue to use subprocess. However the current subprocess wrapper implementation lives in vboot_reference which is a git sub-module of coreboot. This causes all sorts of grief keeping a subprocess ABI stable from vboot_reference when the rest of vboot_reference builds of HEAD of the flashrom tree (i.e., using unreleased libflashrom functions). In order to not keep finding ourseleves in a bind between the two separately moving trees with different build environments, decouple elogtool with its own mini copy of flashrom subprocess wrapping logic. Squash in, util/cbfstool/elogtool.c: Convert args into struct in flashrom helper vboot signatures for flashrom r/w helpers changed in the upstream commit bd2971326ee94fc5. Reflect the change here to allow vboot ref and coreboot to realign. BUG=b:207808292,b:231152447 TEST=builds with vboot_ref uprev. Change-Id: I04925e4d9a44b52e4a6fb6f9cec332cab2c7c725 Signed-off-by: Edward O'Callaghan Reviewed-on: https://review.coreboot.org/c/coreboot/+/65055 Tested-by: build bot (Jenkins) Reviewed-by: Julius Werner --- util/cbfstool/Makefile.inc | 1 + util/cbfstool/elogtool.c | 14 ++-- util/cbfstool/flashrom.c | 143 +++++++++++++++++++++++++++++++++++++ util/cbfstool/flashrom.h | 33 +++++++++ 4 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 util/cbfstool/flashrom.c create mode 100644 util/cbfstool/flashrom.h diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc index f3e6c75dce..7b2d75d2bb 100644 --- a/util/cbfstool/Makefile.inc +++ b/util/cbfstool/Makefile.inc @@ -95,6 +95,7 @@ elogobj += eventlog.o elogobj += valstr.o elogobj += elog.o elogobj += common.o +elogobj += flashrom.o include $(top)/util/cbfstool/fpt_formats/Makefile.inc cse_fpt_obj := diff --git a/util/cbfstool/elogtool.c b/util/cbfstool/elogtool.c index 253e2ee95a..1e9b6605f9 100644 --- a/util/cbfstool/elogtool.c +++ b/util/cbfstool/elogtool.c @@ -79,15 +79,10 @@ static void usage(char *invoked_as) static int elog_read(struct buffer *buffer, const char *filename) { if (filename == NULL) { - uint8_t *buf; - uint32_t buf_size; - - if (flashrom_read(FLASHROM_PROGRAMMER_INTERNAL_AP, ELOG_RW_REGION_NAME, - &buf, &buf_size) != VB2_SUCCESS) { + if (flashrom_host_read(buffer, ELOG_RW_REGION_NAME) != 0) { fprintf(stderr, "Could not read RW_ELOG region using flashrom\n"); return ELOGTOOL_EXIT_READ_ERROR; } - buffer_init(buffer, NULL, buf, buf_size); } else if (buffer_from_file(buffer, filename) != 0) { fprintf(stderr, "Could not read input file: %s\n", filename); return ELOGTOOL_EXIT_READ_ERROR; @@ -106,11 +101,10 @@ static int elog_read(struct buffer *buffer, const char *filename) * If filename is NULL, it saves the buffer using flashrom. * Otherwise, it saves the buffer in the given filename. */ -static int elog_write(struct buffer *buf, const char *filename) +static int elog_write(struct buffer *buffer, const char *filename) { if (filename == NULL) { - if (flashrom_write(FLASHROM_PROGRAMMER_INTERNAL_AP, ELOG_RW_REGION_NAME, - buffer_get(buf), buffer_size(buf)) != VB2_SUCCESS) { + if (flashrom_host_write(buffer, ELOG_RW_REGION_NAME) != 0) { fprintf(stderr, "Failed to write to RW_ELOG region using flashrom\n"); return ELOGTOOL_EXIT_WRITE_ERROR; @@ -118,7 +112,7 @@ static int elog_write(struct buffer *buf, const char *filename) return ELOGTOOL_EXIT_SUCCESS; } - if (buffer_write_file(buf, filename) != 0) { + if (buffer_write_file(buffer, filename) != 0) { fprintf(stderr, "Failed to write to file %s\n", filename); return ELOGTOOL_EXIT_WRITE_ERROR; } diff --git a/util/cbfstool/flashrom.c b/util/cbfstool/flashrom.c new file mode 100644 index 0000000000..64f73e420b --- /dev/null +++ b/util/cbfstool/flashrom.c @@ -0,0 +1,143 @@ +/* Copyright 2020 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +/* For strdup */ +#define _POSIX_C_SOURCE 200809L +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "common.h" /* from cbfstool for buffer API. */ +#include "subprocess.h" /* from vboot_reference */ +#include "flashrom.h" + +#define FLASHROM_EXEC_NAME "flashrom" +#define FLASHROM_PROGRAMMER_INTERNAL_AP "host" + +/** + * Helper to create a temporary file. + * + * @param path_out An output pointer for the filename. Caller should free. + * + * @return 0 on success, -1 for file open error, or -2 for write error. + */ +static int create_temp_file(char **path_out) +{ + int fd; + int rv; + char *path; + mode_t umask_save; + +#if defined(__FreeBSD__) +#define P_tmpdir "/tmp" +#endif + *path_out = NULL; + path = strdup(P_tmpdir "/flashrom.XXXXXX"); + /* Set the umask before mkstemp for security considerations. */ + umask_save = umask(077); + fd = mkstemp(path); + umask(umask_save); + if (fd < 0) { + rv = -1; + goto fail; + } + + close(fd); + *path_out = path; + + return 0; +fail: + free(path); + return rv; +} + +static int run_flashrom(const char *const argv[]) +{ + int status = subprocess_run(argv, &subprocess_null, &subprocess_null, + &subprocess_null); + if (status) { + fprintf(stderr, "Flashrom invocation failed (exit status %d):", + status); + for (const char *const *argp = argv; *argp; argp++) + fprintf(stderr, " %s", *argp); + fprintf(stderr, "\n"); + return -1; + } + + return 0; +} + +int flashrom_host_read(struct buffer *buffer, const char *region) +{ + char *tmpfile; + char region_param[PATH_MAX]; + int rv; + + if (create_temp_file(&tmpfile) != 0) + return -1; + if (region) + snprintf(region_param, sizeof(region_param), "%s:%s", region, + tmpfile); + const char *const argv[] = { + FLASHROM_EXEC_NAME, + "-p", + FLASHROM_PROGRAMMER_INTERNAL_AP, + "-r", + region ? "-i" : tmpfile, + region ? region_param : NULL, + NULL, + }; + rv = run_flashrom(argv); + if (!rv) + rv = buffer_from_file(buffer, tmpfile); + + unlink(tmpfile); + free(tmpfile); + + return rv; +} + +int flashrom_host_write(struct buffer *buffer, const char *region) +{ + char *tmpfile; + char region_param[PATH_MAX]; + int rv; + + if (create_temp_file(&tmpfile) != 0) + return -1; + if (buffer_write_file(buffer, tmpfile) != 0) { + rv = -2; + goto fail; + } + + if (region) + snprintf(region_param, sizeof(region_param), "%s:%s", region, + tmpfile); + const char *const argv[] = { + FLASHROM_EXEC_NAME, + "-p", + FLASHROM_PROGRAMMER_INTERNAL_AP, + "--noverify-all", + "-w", + region ? "-i" : tmpfile, + region ? region_param : NULL, + NULL, + }; + + rv = run_flashrom(argv); + +fail: + unlink(tmpfile); + free(tmpfile); + + return rv; +} diff --git a/util/cbfstool/flashrom.h b/util/cbfstool/flashrom.h new file mode 100644 index 0000000000..0db2194973 --- /dev/null +++ b/util/cbfstool/flashrom.h @@ -0,0 +1,33 @@ +/* Copyright 2020 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + * + * Host utilities to execute flashrom command. + */ + +#include +#include "common.h" /* from cbfstool for buffer API. */ + +/** + * Read using flashrom into an allocated buffer. + * + * @param buffer The parameter that contains the buffer to use + * in the read operation. + * @param region The name of the fmap region to read, or NULL to + * read the entire flash chip. + * + * @return 0 on success, or < 0 on error. + */ +int flashrom_host_read(struct buffer *buffer, const char *region); + +/** + * Write using flashrom from a buffer. + * + * @param buffer The parameter that contains the buffer to use + * in the write operation. + * @param regions The name of the fmap region to write, or NULL to + * write the entire flash chip. + * + * @return 0 on success, or < 0 on error. + */ +int flashrom_host_write(struct buffer *buffer, const char *region);