nvmutil: fix rval in close_files

Signed-off-by: Leah Rowe <leah@libreboot.org>

util/nvmutil/nvmutil.c

@@ -333,6 +333,7 @@ static void check_bin(size_t a, const char *a_name);
 static void rw_gbe_file_part(size_t p, int rw_type,
     const char *rw_type_str);
 static void check_written_part(size_t p);
+static void report_io_err_rw(void);
 static u8 *gbe_mem_offset(size_t part, const char *f_op);
 static off_t gbe_file_offset(size_t part, const char *f_op);
 static off_t gbe_x_offset(size_t part, const char *f_op,
@@ -354,8 +355,8 @@ static int try_err(int loop_err, int errval);
 /*
  * Error handling and cleanup
  */
+static int close_files(void);
 static void err(int nvm_errval, const char *msg, ...);
-static void close_files(void);
 static const char *getnvmprogname(void);
 static void usage(int usage_exit);
 
@@ -408,8 +409,9 @@ static const char *rname = NULL;
  *
  * The code will handle this properly.
  */
-static u8 buf[GBE_FILE_SIZE];
-static u8 pad[GBE_PART_SIZE]; /* the file that wouldn't die */
+static u8 real_buf[GBE_FILE_SIZE];
+static u8 pad[GBE_FILE_SIZE]; /* the file that wouldn't die */
+static u8 *buf = real_buf;
 
 static ushort mac_buf[3];
 static off_t gbe_file_size;
@@ -604,6 +606,13 @@ typedef char bool_part_invert[(PART_INVERT==1)?1:-1];
 
 static int use_prng = 0;
 
+static int io_err_gbe = 0;
+static int rw_check_err_read[] = {0, 0};
+static int rw_check_partial_read[] = {0, 0};
+static int rw_check_bad_part[] = {0, 0};
+
+static int post_rw_checksum[] = {0, 0};
+
 int
 main(int argc, char *argv[])
 {
@@ -676,6 +685,7 @@ main(int argc, char *argv[])
 	run_cmd(cmd_index);
 
 	if (command[cmd_index].flags == O_RDWR) {
+
 		write_gbe_file();
 
 		/*
@@ -688,9 +698,15 @@ main(int argc, char *argv[])
 
 		check_written_part(0);
 		check_written_part(1);
+
+		report_io_err_rw();
+
+		if (io_err_gbe)
+			err(EIO, "%s: bad write", fname);
 	}
 
-	close_files();
+	if (close_files() == -1)
+		err(EIO, "%s: close", fname);
 
 	return EXIT_SUCCESS;
 }
@@ -1542,6 +1558,7 @@ check_written_part(size_t p)
 	size_t gbe_rw_size;
 	u8 *mem_offset;
 	off_t file_offset;
+	u8 *buf_restore;
 
 	if (!part_modified[p])
 		return;
@@ -1552,20 +1569,84 @@ check_written_part(size_t p)
 	mem_offset = gbe_mem_offset(p, "pwrite");
 	file_offset = (off_t)gbe_file_offset(p, "pwrite");
 
+	memset(pad, 0xff, sizeof(pad));
+
 	r = rw_gbe_file_exact(gbe_fd, pad,
 	    gbe_rw_size, file_offset, IO_PREAD);
 
 	if (r == -1)
-		err(errno, "%s: pread: part %lu (post-verification)",
-		    fname, (ulong)p);
+		rw_check_err_read[p] = io_err_gbe = 1;
+	else if ((size_t)r != gbe_rw_size)
+		rw_check_partial_read[p] = io_err_gbe = 1;
+	else if (memcmp(mem_offset, pad, gbe_rw_size) != 0)
+		rw_check_bad_part[p] = io_err_gbe = 1;
 
-	if ((size_t)r != gbe_rw_size)
-		err(EIO, "%s: partial pread: part %lu (post-verification)",
-		    fname, (ulong)p);
+	if (rw_check_err_read[p] ||
+	    rw_check_partial_read[p])
+		return;
 
-	if (memcmp(mem_offset, pad, gbe_rw_size) != 0)
-		err(EIO, "%s: pwrite: corrupt write on part %lu",
-		    fname, (ulong)p);
+	/*
+	 * We only load one part on-file, into memory but
+	 * always at offset zero, for post-write checks.
+	 * That's why we hardcode good_checksum(0).
+	 */
+	buf_restore = buf;
+	buf = pad;
+	post_rw_checksum[p] = good_checksum(0);
+	buf = buf_restore;
+}
+
+static void
+report_io_err_rw(void)
+{
+	size_t p;
+
+	if (!io_err_gbe)
+		return;
+
+	for (p = 0; p < 2; p++) {
+		if (!part_modified[p])
+			continue;
+
+		if (rw_check_err_read[p])
+			fprintf(stderr,
+			    "%s: pread: p%lu (post-verification)\n",
+			    fname, (ulong)p);
+		if (rw_check_partial_read[p])
+			fprintf(stderr,
+			    "%s: partial pread: p%lu (post-verification)\n",
+			    fname, (ulong)p);
+		if (rw_check_bad_part[p])
+			fprintf(stderr,
+			    "%s: pwrite: corrupt write on p%lu\n",
+			    fname, (ulong)p);
+
+		if (rw_check_err_read[p] ||
+		    rw_check_partial_read[p]) {
+			fprintf(stderr,
+			    "%s: p%lu: skipped checksum verification "
+			    "(because read failed)\n",
+			    fname, (ulong)p);
+
+			continue;
+		}
+
+		fprintf(stderr, "%s: ", fname);
+
+		if (post_rw_checksum[p])
+			fprintf(stderr, "GOOD");
+		else
+			fprintf(stderr, "BAD");
+
+		fprintf(stderr, " checksum in p%lu on-disk.\n",
+		    (ulong)p);
+
+		if (post_rw_checksum[p]) {
+			fprintf(stderr,
+			    "    This does NOT mean it's safe. it may be\n"
+			    "    salvageable if you use the cat feature.\n");
+		}
+	}
 }
 
 /*
@@ -1840,8 +1921,7 @@ try_rw_again:
 	saved_errno = errno;
 	if (lseek_loop(fd, off_orig, SEEK_SET,
 	    loop_eagain, loop_eintr) == (off_t)-1) {
-		if (r < 0)
-			errno = saved_errno;
+		errno = saved_errno;
 		return -1;
 	}
 	errno = saved_errno;
@@ -1922,17 +2002,45 @@ try_err(int loop_err, int errval)
 	return -1;
 }
 
+static int
+close_files(void)
+{
+	int close_err_gbe = 0;
+	int close_err_rand = 0;
+	int saved_errno = errno;
+
+	if (gbe_fd > -1) {
+		if (close(gbe_fd) == -1)
+			close_err_gbe = errno;
+		gbe_fd = -1;
+	}
+
+	if (urandom_fd > -1) {
+		if (close(urandom_fd) == -1)
+			close_err_rand = errno;
+		urandom_fd = -1;
+	}
+
+	if (saved_errno)
+		errno = saved_errno;
+
+	if (close_err_gbe || close_err_rand)
+		return -1;
+
+	return 0;
+}
+
 static void
 err(int nvm_errval, const char *msg, ...)
 {
 	va_list args;
 
-	if (nvm_errval >= 0) {
-		close_files();
-		errno = nvm_errval;
-	}
 	if (errno <= 0)
 		errno = ECANCELED;
+	if (!errno)
+		errno = nvm_errval;
+
+	(void)close_files();
 
 	fprintf(stderr, "%s: ", getnvmprogname());
 
@@ -1946,22 +2054,6 @@ err(int nvm_errval, const char *msg, ...)
 	exit(EXIT_FAILURE);
 }
 
-static void
-close_files(void)
-{
-	if (gbe_fd > -1) {
-		if (close(gbe_fd) == -1)
-			err(-1, "%s: close failed", fname);
-		gbe_fd = -1;
-	}
-
-	if (urandom_fd > -1) {
-		if (close(urandom_fd) == -1)
-			err(-1, "%s: close failed", rname);
-		urandom_fd = -1;
-	}
-}
-
 static const char *
 getnvmprogname(void)
 {