diff options
author | Nicolas Boichat <drinkcat@chromium.org> | 2021-04-08 11:46:26 +0800 |
---|---|---|
committer | Jack Rosenthal <jrosenth@chromium.org> | 2021-04-08 16:46:31 +0000 |
commit | 70838cc1a1294f1f53d00c08d3ee7616db073e8e (patch) | |
tree | 540f64491237d9a9fde2d43e51a175d906897bc9 /cgpt | |
parent | 55ec0755959a1f130684e637dff29ea340dc41f8 (diff) | |
download | vboot-70838cc1a1294f1f53d00c08d3ee7616db073e8e.tar.gz |
cgpt: Use subprocess_run to call flashrom with 1>/dev/nullstabilize-quickfix-13904.98.Bstabilize-13904.67.Bstabilize-13904.66.Bstabilize-13904.62.Bstabilize-13904.59.Bstabilize-13904.58.Bstabilize-13904.55.Bstabilize-13904.49.Bstabilize-13904.48.Bstabilize-13904.47.Bstabilize-13904.44.Bstabilize-13904.43.Bstabilize-13904.42.Bstabilize-13904.41.Bstabilize-13904.34.Brelease-R91-13904.B
Closing fd=1 is a bad idea, as flashrom will then reuse fd=1 for
the MTD device and directly write to it.
Luckily, we have a subprocess_run function in vboot that does
what we need. There will be some cleanup required after this,
but hopefully this is enough to clear the P0 on hand.
BUG=b:184559695
TEST=`cgpt find -t kernel` (fails?!), but at least does not
corrupt flash:
`flashrom -r /usr/local/x.bin && hexdump -C /usr/local/x.bin | head`
BRANCH=none
Change-Id: Ia82ed7966ea66274f72fe21eca5241633ffbdb5c
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2812630
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Jack Rosenthal <jrosenth@chromium.org>
Diffstat (limited to 'cgpt')
-rw-r--r-- | cgpt/cgpt_nor.c | 75 |
1 files changed, 56 insertions, 19 deletions
diff --git a/cgpt/cgpt_nor.c b/cgpt/cgpt_nor.c index fd184463..40306676 100644 --- a/cgpt/cgpt_nor.c +++ b/cgpt/cgpt_nor.c @@ -24,6 +24,7 @@ #include "cgpt.h" #include "cgpt_nor.h" +#include "subprocess.h" static const char FLASHROM_PATH[] = "/usr/sbin/flashrom"; @@ -48,6 +49,7 @@ int GetMtdSize(const char *mtd_device, uint64_t *size) { return ret; } +// TODO(b:184559695): Remove these functions and use subprocess_run everywhere. int ForkExecV(const char *cwd, const char *const argv[]) { pid_t pid = fork(); if (pid == -1) { @@ -212,27 +214,42 @@ int ReadNorFlash(char *temp_dir_template) { // Read RW_GPT section from NOR flash to "rw_gpt". ret++; - int fd_flags = fcntl(1, F_GETFD); - // Close stdout on exec so that flashrom does not muck up cgpt's output. - if (0 != fcntl(1, F_SETFD, FD_CLOEXEC)) - Warning("Can't stop flashrom from mucking up our output\n"); - if (ForkExecL(temp_dir_template, FLASHROM_PATH, "-i", "RW_GPT:rw_gpt", "-r", - NULL) != 0) { + + // TODO(b:184559695): Add parameter to subprocess_run to change directory + // before exec. Also, NULL parameter is a glibc extension that _might_ + // break FreeBSD. + char *cwd = getcwd(NULL, 0); + if (!cwd) { + Error("Cannot get current directory.\n"); + return ret; + } + if (chdir(temp_dir_template) < 0) { + Error("Cannot change directory.\n"); + goto out_free; + } + const char *const argv[] = {FLASHROM_PATH, "-i", "RW_GPT:rw_gpt", "-r"}; + // Redirect stdout to /dev/null so that flashrom does not muck up cgpt's + // output. + if (subprocess_run(argv, &subprocess_null, &subprocess_null, NULL) != 0) { Error("Cannot exec flashrom to read from RW_GPT section.\n"); RemoveDir(temp_dir_template); } else { ret = 0; } + if (chdir(cwd) < 0) { + Error("Cannot change directory back to original.\n"); + goto out_free; + } - // Restore stdout flags - if (0 != fcntl(1, F_SETFD, fd_flags)) - Warning("Can't restore stdout flags\n"); +out_free: + free(cwd); return ret; } // Write "rw_gpt" back to NOR flash. We write the file in two parts for safety. int WriteNorFlash(const char *dir) { int ret = 0; + ret++; if (split_gpt(dir, "rw_gpt") != 0) { Error("Cannot split rw_gpt in two.\n"); @@ -240,26 +257,46 @@ int WriteNorFlash(const char *dir) { } ret++; int nr_fails = 0; - int fd_flags = fcntl(1, F_GETFD); - // Close stdout on exec so that flashrom does not muck up cgpt's output. - if (0 != fcntl(1, F_SETFD, FD_CLOEXEC)) - Warning("Can't stop flashrom from mucking up our output\n"); - if (ForkExecL(dir, FLASHROM_PATH, "-i", "RW_GPT_PRIMARY:rw_gpt_1", - "-w", "--fast-verify", NULL) != 0) { + + // TODO(b:184559695): Add parameter to subprocess_run to change directory + // before exec. Also, NULL parameter is a glibc extension that _might_ + // break FreeBSD. + char *cwd = getcwd(NULL, 0); + if (!cwd) { + Error("Cannot get current directory.\n"); + return ret; + } + if (chdir(dir) < 0) { + Error("Cannot change directory.\n"); + goto out_free; + } + const char *const argv1[] = {FLASHROM_PATH, "-i", "RW_GPT_PRIMARY:rw_gpt_1", + "-w", "--fast-verify"}; + // Redirect stdout to /dev/null so that flashrom does not muck up cgpt's + // output. + if (subprocess_run(argv1, &subprocess_null, &subprocess_null, NULL) != 0) { Warning("Cannot write the 1st half of rw_gpt back with flashrom.\n"); nr_fails++; } - if (ForkExecL(dir, FLASHROM_PATH, "-i", "RW_GPT_SECONDARY:rw_gpt_2", - "-w", "--fast-verify", NULL) != 0) { + const char *const argv2[] = {FLASHROM_PATH, "-i", "RW_GPT_SECONDARY:rw_gpt_2", + "-w", "--fast-verify"}; + // Redirect stdout to /dev/null so that flashrom does not muck up cgpt's + // output. + if (subprocess_run(argv2, &subprocess_null, &subprocess_null, NULL) != 0) { Warning("Cannot write the 2nd half of rw_gpt back with flashrom.\n"); nr_fails++; } - if (0 != fcntl(1, F_SETFD, fd_flags)) - Warning("Can't restore stdout flags\n"); + if (chdir(cwd) < 0) { + Error("Cannot change directory back to original.\n"); + goto out_free; + } switch (nr_fails) { case 0: ret = 0; break; case 1: Warning("It might still be okay.\n"); break; case 2: Error("Cannot write both parts back with flashrom.\n"); break; } + +out_free: + free(cwd); return ret; } |