Change in flashrom[master]: cli_classic: prevent corruption of flash when stdout/stderr is closed

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52215 )

Change subject: cli_classic: prevent corruption of flash when stdout/stderr is closed
......................................................................

cli_classic: prevent corruption of flash when stdout/stderr is closed

While it's not posixly-correct, it's possible that a user, script, or
application may attempt to start flashrom with stdout or stderr
closed.  It's possible that we'll get a file descriptor of 1 or 2 when
opening a flash device (such as Linux MTD), and flashrom will send
garbage debug logs to the flash:

# bash -c "exec >&- flashrom ..."

Observed corruption:
  43 40 45 42 45 44 00 00 00 00 00 00 01 00 00 00 |C@EBED..........|
  00 02 00 00 63 65 73 73 66 75 6c 6c 79 0a 46 6f |....cessfully.Fo|
  75 6e 64 20 50 72 6f 67 72 61 6d 6d 65 72 20 66 |und Programmer f|
  6c 61 73 68 20 63 68 69 70 20 22 4f 70 61 71 75 |lash chip "Opaqu|
  65 20 66 6c 61 73 68 20 63 68 69 70 22 20 28 38 |e flash chip" (8|
  31 39 32 20 6b 42 2c 20 50 72 6f 67 72 61 6d 6d |192 kB, Programm|
  65 72 2d 73 70 65 63 69 66 69 63 29 20 6d 61 70 |er-specific) map|
  70 65 64 20 61 74 20 70 68 79 73 69 63 61 6c 20 |ped at physical |
  61 64 64 72 65 73 73 20 30 78 30 30 30 30 30 30 |address 0x000000|
  30 30 2e 0a ff ff ff ff ff ff ff ff ff ff ff ff |00..............|
  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
  ...

While for most applications, closing stdout or stderr would just lead
to obsure bugs, for flashrom, we should have extra safety guards, as
this could mean that we might be bricking a device instead.

Add a basic safety check.

Signed-off-by: Jack Rosenthal 
Change-Id: I751c9dd88ad1d30283b94bd2185b4f8f25569c8f
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52215
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
M cli_classic.c
1 file changed, 23 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/cli_classic.c b/cli_classic.c
index 3dbaec2..3160d76 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -22,6 +22,7 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <string.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <getopt.h>
 #include "flash.h"
@@ -110,6 +111,18 @@
    return 0;
 }

+/* Ensure a file is open by means of fstat */
+static bool check_file(FILE *file)
+{
+#ifndef STANDALONE
+	struct stat statbuf;
+
+	if (fstat(fileno(file), &statbuf) < 0)
+		return false;
+#endif /* !STANDALONE */
+	return true;
+}
+
 static int parse_wp_range(unsigned int *start, unsigned int *len)
 {
    char *endptr = NULL, *token = NULL;
@@ -219,7 +232,16 @@
    struct layout_include_args *include_args = NULL;
    char *wp_mode_opt = NULL;

-	flashrom_set_log_callback((flashrom_log_callback *)&flashrom_print_cb);
+	/*
+	 * Safety-guard against a user who has (mistakenly) closed
+	 * stdout or stderr before exec'ing flashrom.  We disable
+	 * logging in this case to prevent writing log data to a flash
+	 * chip when a flash device gets opened with fd 1 or 2.
+	 */
+	if (check_file(stdout) && check_file(stderr)) {
+		flashrom_set_log_callback(
+			(flashrom_log_callback *)&flashrom_print_cb);
+	}

print_version();
    print_banner();