cgpt: Use subprocess_run to call flashrom with 1>/dev/null
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>
diff --git a/cgpt/cgpt_nor.c b/cgpt/cgpt_nor.c
index fd18446..4030667 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 @@
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 @@
// 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 @@
}
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;
}
