1 files changed, 299 insertions, 109 deletions

diff --git a/util/nvmutil/nvmutil.c b/util/nvmutil/nvmutil.c
index a468fd6a..319fe552 100644
--- a/util/nvmutil/nvmutil.c
+++ b/util/nvmutil/nvmutil.c
@@ -4,30 +4,34 @@
 
 #include <sys/stat.h>
 
-#include <err.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <stdarg.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
+static void reset_global_state(void);
 static void set_cmd(int, char **);
 static void check_cmd_args(int, char **);
 static void set_io_flags(int, char **);
 static void open_files(void);
-static void checkdir(const char *);
 static void xopen(int *, const char *, int, struct stat *);
 static void read_gbe(void);
 static void read_gbe_part(int, int);
 static void cmd_setmac(void);
 static void parse_mac_string(void);
-static void set_mac_byte(int, uint64_t *);
+static void set_mac_byte(int);
 static void check_mac_separator(int);
-static void set_mac_nib(int, int, uint8_t *);
+static void set_mac_nib(int, int);
 static uint8_t hextonum(char);
 static uint8_t rhex(void);
+static void read_file_PERFECTLY_or_die(int, void *, size_t,
+    off_t, const char *, const char *);
+static int check_read_or_die(const char *,
+    ssize_t, size_t, int, const char *);
 static int write_mac_part(int);
 static void cmd_dump(void);
 static void print_mac_address(int);
@@ -43,9 +47,10 @@ static void check_bound(int, int);
 static void write_gbe(void);
 static void write_gbe_part(int);
 static void swap(int);
-static void usage(const char *);
-static void err_if(int);
-static int set_err(int);
+static void usage(void);
+static void err(int, const char *, ...);
+static const char *getnvmprogname(void);
+static void set_err(int);
 
 #define NVM_CHECKSUM 0xBABA
 #define NVM_CHECKSUM_WORD 0x3F
@@ -56,15 +61,17 @@ static int set_err(int);
 #define SIZE_16KB 0x4000
 #define SIZE_128KB 0x20000
 
+#define MAX_RETRY_READ 30
+
 #define items(x) (sizeof((x)) / sizeof((x)[0]))
 
 static uint8_t buf[SIZE_8KB];
 static uint16_t macbuf[3];
-static size_t partsize;
+static off_t partsize;
 
 static int flags;
-static int rfd;
-static int fd;
+static int rfd = -1;
+static int fd = -1;
 static int part;
 static int invert;
 static int part_modified[2];
@@ -72,6 +79,7 @@ static int part_modified[2];
 static const char *mac = NULL;
 static const char *rmac = "xx:xx:xx:xx:xx:xx";
 static const char *fname = "";
+static const char *argv0;
 
 struct op {
 	const char *str;
@@ -92,73 +100,119 @@ static void (*cmd)(void) = NULL;
 int
 main(int argc, char *argv[])
 {
+	argv0 = argv[0];
 	if (argc < 2)
-		usage(argv[0]);
+		usage();
+
+	reset_global_state();
 	fname = argv[1];
+
 #ifdef __OpenBSD__
-	err_if(pledge("stdio rpath wpath unveil", NULL) == -1);
-	err_if(unveil("/dev/urandom", "r") == -1);
+	if (pledge("stdio rpath wpath unveil", NULL) == -1)
+		err(ECANCELED, "pledge");
+	if (unveil("/dev/urandom", "r") == -1)
+		err(ECANCELED, "unveil '/dev/urandom'");
 #endif
+
 	set_cmd(argc, argv);
 	check_cmd_args(argc, argv);
 	set_io_flags(argc, argv);
+
 #ifdef __OpenBSD__
 	if (flags == O_RDONLY) {
-		err_if(unveil(fname, "r") == -1);
-		err_if(unveil(NULL, NULL) == -1);
-		err_if(pledge("stdio rpath", NULL) == -1);
+		if (unveil(fname, "r") == -1)
+			err(ECANCELED, "unveil ro '%s'", fname);
+		if (unveil(NULL, NULL) == -1)
+			err(ECANCELED, "unveil block (ro)");
+		if (pledge("stdio rpath", NULL) == -1)
+			err(ECANCELED, "pledge ro (kill unveil)");
 	} else {
-		err_if(unveil(fname, "rw") == -1);
-		err_if(unveil(NULL, NULL) == -1);
-		err_if(pledge("stdio rpath wpath", NULL) == -1);
+		if (unveil(fname, "rw") == -1)
+			err(ECANCELED, "unveil rw '%s'", fname);
+		if (unveil(NULL, NULL) == -1)
+			err(ECANCELED, "unveil block (rw)");
+		if (pledge("stdio rpath wpath", NULL) == -1)
+			err(ECANCELED, "pledge rw (kill unveil)");
 	}
 #endif
+
 	open_files();
+
 #ifdef __OpenBSD__
-	err_if(pledge("stdio", NULL) == -1);
+	if (pledge("stdio", NULL) == -1)
+		err(ECANCELED, "pledge stdio (main)");
 #endif
+
 	read_gbe();
 	(*cmd)();
 	write_gbe();
 
-	err_if(close(fd) == -1);
+	if (close(fd) == -1)
+		err(ECANCELED, "close '%s'", fname);
+	if (close(rfd) == -1)
+		err(ECANCELED, "close '/dev/urandom'");
+
+	if (cmd != cmd_dump) {
+		if (errno)
+			err(ECANCELED, "Unhandled error on exit");
+	}
+
+	if (errno)
+		return EXIT_FAILURE;
+	else
+		return EXIT_SUCCESS;
+}
+
+/*
+ * Currently redundant, because the program only runs
+ * once, but I plan to expand this tool so that it can
+ * work on multiple files, using getop switches as args.
+ */
+static void
+reset_global_state(void)
+{
+	errno = 0;
+
+	mac = NULL;
+	invert = 0;
+	part_modified[0] = 0;
+	part_modified[1] = 0;
+	fname = "";
+	cmd = NULL;
+	fd = -1;
+	rfd = -1;
+	part = 0;
 
-	err_if((errno != 0) && (cmd != cmd_dump));
-	return errno ? EXIT_FAILURE : EXIT_SUCCESS;
+	memset(macbuf, 0, sizeof(macbuf));
+	memset(buf, 0, sizeof(buf));
 }
 
 static void
 set_cmd(int argc, char *argv[])
 {
 	size_t i;
-	const char *arg_cmd;
 
 	if (argc == 2) {
 		cmd = cmd_setmac;
 		return;
 	}
 
-	arg_cmd = argv[2];
-
 	for (i = 0; (i < items(ops)) && (cmd == NULL); i++) {
-		if (strcmp(arg_cmd, ops[i].str) != 0)
+		if (strcmp(argv[2], ops[i].str) != 0)
 			continue;
 		if (argc >= ops[i].args) {
 			cmd = ops[i].cmd;
 			break;
 		}
-		err(set_err(EINVAL), "Too few args on command '%s'",
-		    ops[i].str);
+		err(EINVAL, "Too few args: command '%s'", ops[i].str);
 	}
 }
 
 static void
 check_cmd_args(int argc, char *argv[])
 {
-	const char *arg_cmd = argv[2];
-
 	if ((cmd == NULL) && (argc > 2)) { /* nvm gbe [MAC] */
-		mac = arg_cmd;
+		mac = argv[2];
 		cmd = cmd_setmac;
 	} else if (cmd == cmd_setmac) { /* nvm gbe setmac [MAC] */
 		mac = rmac; /* random MAC */
@@ -167,21 +221,19 @@ check_cmd_args(int argc, char *argv[])
 	} else if ((cmd != NULL) && (argc > 3)) { /* user-supplied partnum */
 		part = argv[3][0] - '0';
 		if (!((part == 0 || part == 1) && argv[3][1] == '\0'))
-			err(set_err(EINVAL), "Bad partnum: %s", argv[3]);
+			err(EINVAL, "Bad partnum: %s", argv[3]);
 	}
 
 	if (cmd == NULL)
-		err(set_err(EINVAL), "Bad command");
+		err(EINVAL, "Bad command");
 }
 
 static void
 set_io_flags(int argc, char *argv[])
 {
-	const char *arg_cmd = argv[2];
-
 	flags = O_RDWR;
 	if (argc > 2) {
-		if (strcmp(arg_cmd, "dump") == 0)
+		if (strcmp(argv[2], "dump") == 0)
 			flags = O_RDONLY;
 	}
 }
@@ -192,9 +244,6 @@ open_files(void)
 	struct stat st;
 	struct stat st_rfd;
 
-	checkdir("/dev/urandom");
-	checkdir(fname);
-
 	xopen(&rfd, "/dev/urandom", O_RDONLY, &st_rfd);
 	xopen(&fd, fname, flags, &st);
 
@@ -205,28 +254,18 @@ open_files(void)
 		partsize = st.st_size >> 1;
 		break;
 	default:
-		err(set_err(ECANCELED), "Invalid file size (not 8/16/128KiB)");
+		err(ECANCELED, "File size must be 8KB, 16KB or 128KB");
 		break;
 	}
 }
 
 static void
-checkdir(const char *path)
-{
-	struct stat st;
-	if (stat(path, &st) == -1)
-		err(set_err(ECANCELED), "%s", path);
-	if (S_ISDIR(st.st_mode))
-		err(set_err(EISDIR), "%s", path);
-}
-
-static void
 xopen(int *f, const char *l, int p, struct stat *st)
 {
 	if ((*f = open(l, p)) == -1)
-		err(set_err(ECANCELED), "%s", l);
+		err(ECANCELED, "%s", l);
 	if (fstat(*f, st) == -1)
-		err(set_err(ECANCELED), "%s", l);
+		err(ECANCELED, "%s", l);
 }
 
 static void
@@ -255,11 +294,10 @@ read_gbe(void)
 static void
 read_gbe_part(int p, int invert)
 {
-	if (pread(fd, buf + (SIZE_4KB * (p ^ invert)), SIZE_4KB, p * partsize)
-	    != SIZE_4KB)
-		err(set_err(ECANCELED),
-		    "Can't read %d b from '%s' p%d", SIZE_4KB, fname, p);
-	swap(p ^ invert); /* handle big-endian host CPU */
+	read_file_PERFECTLY_or_die(fd, buf + (SIZE_4KB * (p ^ invert)),
+	    SIZE_4KB, ((off_t)p) * partsize, fname, "pread");
+
+	swap(p ^ invert);
 }
 
 static void
@@ -267,8 +305,8 @@ cmd_setmac(void)
 {
 	int partnum;
 	int mac_updated = 0;
-	parse_mac_string();
 
+	parse_mac_string();
 	printf("MAC address to be written: %s\n", mac);
 
 	for (partnum = 0; partnum < 2; partnum++)
@@ -281,29 +319,27 @@ static void
 parse_mac_string(void)
 {
 	int mac_pos;
-	uint64_t total = 0;
 
 	if (strnlen(mac, 20) != 17)
-		err(set_err(EINVAL), "Invalid MAC address string length");
+		err(EINVAL, "MAC address is the wrong length");
 
 	for (mac_pos = 0; mac_pos < 16; mac_pos += 3)
-		set_mac_byte(mac_pos, &total);
+		set_mac_byte(mac_pos);
 
-	if (total == 0)
-		err(set_err(EINVAL), "Invalid MAC (all-zero MAC address)");
+	if ((macbuf[0] | macbuf[1] | macbuf[2]) == 0)
+		err(EINVAL, "Must not specify all-zeroes MAC address");
 	if (macbuf[0] & 1)
-		err(set_err(EINVAL), "Invalid MAC (multicast bit set)");
+		err(EINVAL, "Must not specify multicast MAC address");
 }
 
 static void
-set_mac_byte(int mac_pos, uint64_t *total)
+set_mac_byte(int mac_pos)
 {
 	int nib;
-	uint8_t h = 0;
 	check_mac_separator(mac_pos);
 
-	for (nib = 0; nib < 2; nib++, *total += h)
-		set_mac_nib(mac_pos, nib, &h);
+	for (nib = 0; nib < 2; nib++)
+		set_mac_nib(mac_pos, nib);
 }
 
 static void
@@ -316,16 +352,17 @@ check_mac_separator(int mac_pos)
 	if ((separator = mac[mac_pos + 2]) == ':')
 		return;
 
-	err(set_err(EINVAL), "Invalid MAC address separator '%c'", separator);
+	err(EINVAL, "Invalid MAC address separator '%c'", separator);
 }
 
 static void
-set_mac_nib(int mac_pos, int nib, uint8_t *h)
+set_mac_nib(int mac_pos, int nib)
 {
+	uint8_t h;
 	int byte = mac_pos / 3;
 
-	if ((*h = hextonum(mac[mac_pos + nib])) > 15)
-		err(set_err(EINVAL), "Invalid character '%c'",
+	if ((h = hextonum(mac[mac_pos + nib])) > 15)
+		err(EINVAL, "Invalid character '%c'",
 		    mac[mac_pos + nib]);
 
 	/* If random, ensure that local/unicast bits are set */
@@ -333,10 +370,10 @@ set_mac_nib(int mac_pos, int nib, uint8_t *h)
 		if ((mac[mac_pos + nib] == '?') ||
 		    (mac[mac_pos + nib] == 'x') ||
 		    (mac[mac_pos + nib] == 'X')) /* random */
-			*h = (*h & 0xE) | 2; /* local, unicast */
+			h = (h & 0xE) | 2; /* local, unicast */
 	}
 
-	macbuf[byte >> 1] |= ((uint16_t ) *h) << ((8 * (byte % 2)) +
+	macbuf[byte >> 1] |= (uint16_t)h << ((8 * (byte % 2)) +
 	    (4 * (nib ^ 1)));
 }
 
@@ -363,12 +400,59 @@ rhex(void)
 
 	if (!n) {
 		n = sizeof(rnum) - 1;
-		err_if(read(rfd, (uint8_t *) &rnum, n + 1) == -1);
+		read_file_PERFECTLY_or_die(rfd, rnum, sizeof(rnum),
+		    0, "/dev/urandom", NULL);
 	}
 
 	return rnum[n--] & 0xf;
 }
 
+static void
+read_file_PERFECTLY_or_die(int fd, void *buf, size_t len,
+    off_t off, const char *path, const char *op)
+{
+	int retry;
+	ssize_t rval;
+
+	for (retry = 0; retry < MAX_RETRY_READ; retry++) {
+		if (op == NULL)
+			rval = read(fd, buf, len);
+		else
+			rval = pread(fd, buf, len, off);
+
+		if (check_read_or_die(path, rval, len, retry,
+		    op ? op : "read"))
+			return;
+	}
+
+	err(EINTR, "%s: max retries exceeded on file: %s",
+	    op ? op : "read", path);
+}
+
+static int
+check_read_or_die(const char *rpath, ssize_t rval, size_t rsize,
+    int retry, const char *readtype)
+{
+	if (rval == (ssize_t)rsize) {
+		errno = 0;
+		return 1; /* Successful read */
+	}
+
+	if (rval != -1)
+		err(ECANCELED, "Short %s, %zd bytes, on file: %s",
+		    readtype, rval, rpath);
+	if (errno != EINTR)
+		err(ECANCELED, "Could not %s file: '%s'", readtype, rpath);
+	if (retry == MAX_RETRY_READ - 1)
+		err(EINTR, "%s: max retries exceeded on file: %s",
+		    readtype, rpath);
+
+	/*
+	 * Bad read, with errno EINTR (syscall interrupted).
+	 */
+	return 0;
+}
+
 static int
 write_mac_part(int partnum)
 {
@@ -426,11 +510,12 @@ hexdump(int partnum)
 {
 	int c;
 	int row;
+	uint16_t val16;
 
 	for (row = 0; row < 8; row++) {
 		printf("%08x ", row << 4);
 		for (c = 0; c < 8; c++) {
-			uint16_t val16 = word((row << 3) + c, partnum);
+			val16 = word((row << 3) + c, partnum);
 			if (c == 4)
 				printf(" ");
 			printf(" %02x %02x", val16 & 0xff, val16 >> 8);
@@ -454,24 +539,57 @@ cmd_setchecksum(void)
 static void
 cmd_brick(void)
 {
-	if (good_checksum(part))
-		set_word(NVM_CHECKSUM_WORD, part,
-		    ((word(NVM_CHECKSUM_WORD, part)) ^ 0xFF));
+	uint16_t checksum_word;
+
+	if (!good_checksum(part)) {
+		err(ECANCELED,
+		    "Part %d checksum already invalid in file '%s'",
+		    part, fname);
+	}
+
+	/*
+	 * We know checksum_word is valid, so we need only
+	 * flip one bit to invalidate it.
+	 */
+	checksum_word = word(NVM_CHECKSUM_WORD, part);
+	set_word(NVM_CHECKSUM_WORD, part, checksum_word ^ 1);
 }
 
 static void
 cmd_copy(void)
 {
-	err_if(!good_checksum(part ^ 1));
+	if (!good_checksum(part ^ 1))
+		err(ECANCELED, "copy p%d, file '%s'", part ^ 1, fname);
+
+	/*
+	 * SPEED HACK:
+	 *
+	 * read_gbe() already performed the copy,
+	 * by virtue of inverted read. We need
+	 * only set the other part as changed.
+	 */
 	part_modified[part ^ 1] = 1;
 }
 
 static void
 cmd_swap(void)
 {
-	err_if(!(good_checksum(0) || good_checksum(1)));
+	if (!(good_checksum(0) || good_checksum(1)))
+		err(ECANCELED, "swap parts, file '%s'", fname);
+
+	/*
+	 * good_checksum() can set errno, if one
+	 * of the parts is bad. We will reset it.
+	 */
 	errno = 0;
 
+	/*
+	 * SPEED HACK:
+	 *
+	 * read_gbe() already performed the swap,
+	 * by virtue of inverted read. We need
+	 * only set both parts as changed.
+	 */
 	part_modified[1] = part_modified[0] = 1;
 }
 
@@ -488,32 +606,59 @@ good_checksum(int partnum)
 
 	fprintf(stderr, "WARNING: BAD checksum in part %d\n",
 	    partnum ^ invert);
-	(void) set_err(ECANCELED);
+
+	set_err(ECANCELED);
 	return 0;
 }
 
+/*
+ * NOTE: memcpy is a bit sticky with host endianness,
+ *       but we currently use it only when swap has
+ *	 been handled. just be careful about when the
+ *	 swap() function is called.
+ */
+
 static uint16_t
 word(int pos16, int p)
 {
+	uint16_t rval = 0;
+
 	check_bound(pos16, p);
-	return ((uint16_t *) (buf + (SIZE_4KB * p)))[pos16];
+	memcpy(&rval, buf + (SIZE_4KB * p) + (pos16 << 1), sizeof(uint16_t));
+
+	return rval;
 }
 
 static void
 set_word(int pos16, int p, uint16_t val16)
 {
 	check_bound(pos16, p);
+	memcpy(buf + (SIZE_4KB * p) + (pos16 << 1), &val16, sizeof(uint16_t));
+
 	part_modified[p] = 1;
-	((uint16_t *) (buf + (SIZE_4KB * p)))[pos16] = val16;
 }
 
 static void
 check_bound(int c, int p)
 {
+	/*
+	 * NVM_SIZE assumed as the limit, because the
+	 * current design assumes that we will only
+	 * ever modified the NVM area.
+	 *
+	 * The only exception is copy/swap, but these
+	 * do not use word/set_word and therefore do
+	 * not cause check_bound() to be called.
+	 *
+	 * TODO:
+	 * This should be adjusted in the future, if
+	 * we ever wish to work on the extented area.
+	 */
+
 	if ((p != 0) && (p != 1))
-		err(set_err(EINVAL), "check_bound: invalid partnum %d", p);
-	if ((c < 0) || (c >= (SIZE_4KB >> 1)))
-		err(set_err(EINVAL), "check_bound: out of bounds %d", c);
+		err(EINVAL, "check_bound: invalid partnum %d", p);
+	if ((c < 0) || (c >= (NVM_SIZE >> 1)))
+		err(EINVAL, "check_bound: out of bounds %d", c);
 }
 
 static void
@@ -536,27 +681,39 @@ write_gbe_part(int p)
 	swap(p); /* swap bytes on big-endian host CPUs */
 
 	if (pwrite(fd, buf + (SIZE_4KB * p),
-	    SIZE_4KB, p * partsize) != SIZE_4KB) {
-		err(set_err(ECANCELED),
+	    SIZE_4KB, (off_t)p * partsize) != (ssize_t)SIZE_4KB) {
+		err(ECANCELED,
 		    "Can't write %d b to '%s' p%d", SIZE_4KB, fname, p);
 	}
 }
 
+/*
+ * GbE files store bytes in little-endian order.
+ * This function ensures big-endian host CPU support.
+ */
 static void
 swap(int partnum)
 {
+	/*
+	 * NVM_SIZE assumed as the limit; see notes in
+	 * check_bound().
+	 *
+	 * TODO:
+	 * This should be adjusted in the future, if
+	 * we ever wish to work on the extended area.
+	 */
+
 	size_t w;
 	size_t x;
 
-	int e = 1;
 	uint8_t *n = buf + (SIZE_4KB * partnum);
 
-	if (((uint8_t *) &e)[0] == 1)
+	int e = 1;
+	if (*((uint8_t *)&e) == 1)
 		return; /* Little-endian host CPU; no swap needed. */
 
 	/*
 	 * The host CPU stores bytes in big-endian order.
-	 * GbE files store bytes in little-endian order.
 	 * We will therefore reverse the order in memory:
 	 */
 	for (w = 0, x = 1; w < NVM_SIZE; w += 2, x += 2) {
@@ -567,35 +724,68 @@ swap(int partnum)
 }
 
 static void
-usage(const char *util)
+usage(void)
 {
+	const char *util = getnvmprogname();
+
 #ifdef __OpenBSD__
-	err_if(pledge("stdio", NULL) == -1);
+	if (pledge("stdio", NULL) == -1)
+		err(ECANCELED, "pledge");
 #endif
 	fprintf(stderr,
 	    "Modify Intel GbE NVM images e.g. set MAC\n"
 	    "USAGE:\n"
-	    "%s FILE dump\n"
-	    " %s FILE\n # same as setmac without arg\n"
-	    " %s FILE setmac [MAC]\n"
-	    " %s FILE swap\n"
-	    " %s FILE copy 0|1\n"
-	    " %s FILE brick 0|1\n"
-	    " %s FILE setchecksum 0|1\n",
+	    "\t%s FILE dump\n"
+	    "\t%s FILE   # same as setmac without [MAC]\n"
+	    "\t%s FILE setmac [MAC]\n"
+	    "\t%s FILE swap\n"
+	    "\t%s FILE copy 0|1\n"
+	    "\t%s FILE brick 0|1\n"
+	    "\t%s FILE setchecksum 0|1\n",
 	    util, util, util, util, util, util, util);
-	err(set_err(ECANCELED), "Too few arguments");
+
+	err(ECANCELED, "Too few arguments");
 }
 
 static void
-err_if(int x)
+err(int nvm_errval, const char *msg, ...)
 {
-	if (x)
-		err(set_err(ECANCELED), "%s", fname);
+	va_list args;
+
+	fprintf(stderr, "%s: ", getnvmprogname());
+
+	va_start(args, msg);
+	vfprintf(stderr, msg, args);
+	va_end(args);
+
+	set_err(nvm_errval);
+	fprintf(stderr, ": %s", strerror(errno));
+
+	fprintf(stderr, "\n");
+	exit(EXIT_FAILURE);
 }
 
-static int
+static const char *
+getnvmprogname(void)
+{
+	const char *p;
+
+	if ((argv0 == NULL) || (*argv0 == '\0'))
+		return "";
+
+	if ((p = strrchr(argv0, '/')))
+		return p + 1;
+	else
+		return argv0;
+}
+
+static void
 set_err(int x)
 {
-	errno = errno ? errno : x;
-	return EXIT_FAILURE;
+	if (errno)
+		return;
+	if (x)
+		errno = x;
+	else
+		errno = ECANCELED;
 }