util/nvmutil: minor cleanup

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

util/nvmutil/nvmutil.c

@@ -41,52 +41,104 @@ static void cmd_brick(void);
 static void cmd_copy(void);
 static void cmd_swap(void);
 static int good_checksum(int);
-static uint16_t word(int, int);
-static void set_word(int, int, uint16_t);
-static void check_bound(int, int);
+static uint16_t word(size_t, int);
+static void set_word(size_t, int, uint16_t);
+static void check_bound(size_t, int);
 static void write_gbe(void);
 static void write_gbe_part(int);
-static void swap(int);
+static off_t gbe_file_offset(int, const char *);
+static void *gbe_mem_offset(int p, const char *f_op);
+static off_t gbe_x_offset(int, const char *, const char *, off_t, off_t);
 static void usage(void);
 static void err(int, const char *, ...);
 static const char *getnvmprogname(void);
 static void set_err(int);
 
+/*
+ * On the platforms below, we will use arc4random
+ * for random MAC address generation.
+ *
+ * Later on, the code has fallbacks for other systems.
+ */
+#if defined(__OpenBSD__) || defined(__FreeBSD__) || \
+    defined(__NetBSD__) || defined(__APPLE__) || \
+    defined(__DragonFly__)
+#ifndef HAVE_ARC4RANDOM
+#define HAVE_ARC4RANDOM
+#endif
+#endif
+
+/*
+ * Sizes in bytes:
+ */
+#define SIZE_1KB 1024
+#define SIZE_4KB (4 * SIZE_1KB)
+#define SIZE_8KB (8 * SIZE_1KB)
+#define SIZE_16KB (16 * SIZE_1KB)
+#define SIZE_128KB (128 * SIZE_1KB)
+
+/*
+ * First 128 bytes of a GbE part contains
+ * the regular NVM (Non-Volatile-Memory)
+ * area. All of these bytes must add up,
+ * truncated to 0xBABA.
+ *
+ * The full GbE region is 4KB, but only
+ * the first 128 bytes are used here.
+ *
+ * There is a second 4KB part with the same
+ * rules, and it *should* be identical.
+ */
+#define GBE_FILE_SIZE SIZE_8KB /* for buf */
+#define GBE_PART_SIZE (GBE_FILE_SIZE >> 1)
 #define NVM_CHECKSUM 0xBABA
-#define NVM_CHECKSUM_WORD 0x3F
 #define NVM_SIZE 128
+#define NVM_WORDS (NVM_SIZE >> 1)
+#define NVM_CHECKSUM_WORD (NVM_WORDS - 1)
 
-#define SIZE_4KB 0x1000
-#define SIZE_8KB 0x2000
-#define SIZE_16KB 0x4000
-#define SIZE_128KB 0x20000
-
+/*
+ * When reading files, we loop on error EINTR
+ * a maximum number of defined defined thus:
+ */
 #define MAX_RETRY_READ 30
 
+/*
+ * Portably macro based on BSD nitems.
+ * Used to count the number of commands (see below).
+ */
 #define items(x) (sizeof((x)) / sizeof((x)[0]))
 
-static uint8_t buf[SIZE_8KB];
+static const char newrandom[] = "/dev/urandom";
+static const char oldrandom[] = "/dev/random"; /* fallback on OLD unix */
+#ifndef HAVE_ARC4RANDOM
+static const char *rname = NULL;
+#endif
+
+static uint8_t buf[GBE_FILE_SIZE]; /* 8KB */
 static uint16_t macbuf[3];
 static off_t partsize;
 
 static int flags;
+#ifndef HAVE_ARC4RANDOM
 static int rfd = -1;
+#endif
 static int fd = -1;
+static struct stat st;
 static int part;
 static int invert;
 static int part_modified[2];
 
 static const char *mac = NULL;
-static const char *rmac = "xx:xx:xx:xx:xx:xx";
+static const char rmac[] = "xx:xx:xx:xx:xx:xx";
 static const char *fname = "";
 static const char *argv0;
 
-struct op {
+struct commands {
 	const char *str;
 	void (*cmd)(void);
 	int args;
 };
-static const struct op ops[] = {
+static const struct commands command[] = {
 	{ "dump", cmd_dump, 3 },
 	{ "setmac", cmd_setmac, 3 },
 	{ "swap", cmd_swap, 3 },
@@ -110,8 +162,22 @@ main(int argc, char *argv[])
 #ifdef __OpenBSD__
 	if (pledge("stdio rpath wpath unveil", NULL) == -1)
 		err(ECANCELED, "pledge");
+
+	/*
+	 * For restricted filesystem access on early error.
+	 *
+	 * Unveiling the random device early, regardless of
+	 * whether we will use it, prevents operations on any
+	 * GbE files until we permit it, while performing the
+	 * prerequisite error checks.
+	 *
+	 * We don't actually use the random device on platforms
+	 * that have arc4random, which includes OpenBSD.
+	 */
 	if (unveil("/dev/urandom", "r") == -1)
 		err(ECANCELED, "unveil '/dev/urandom'");
+	if (unveil("/dev/random", "r") == -1)
+		err(ECANCELED, "unveil '/dev/random'");
 #endif
 
 	set_cmd(argc, argv);
@@ -149,8 +215,10 @@ main(int argc, char *argv[])
 
 	if (close(fd) == -1)
 		err(ECANCELED, "close '%s'", fname);
+#ifndef HAVE_ARC4RANDOM
 	if (close(rfd) == -1)
-		err(ECANCELED, "close '/dev/urandom'");
+		err(ECANCELED, "close '%s'", rname);
+#endif
 
 	if (cmd != cmd_dump) {
 		if (errno)
@@ -180,7 +248,9 @@ reset_global_state(void)
 	fname = "";
 	cmd = NULL;
 	fd = -1;
+#ifndef HAVE_ARC4RANDOM
 	rfd = -1;
+#endif
 	part = 0;
 
 	memset(macbuf, 0, sizeof(macbuf));
@@ -197,28 +267,28 @@ set_cmd(int argc, char *argv[])
 		return;
 	}
 
-	for (i = 0; (i < items(ops)) && (cmd == NULL); i++) {
-		if (strcmp(argv[2], ops[i].str) != 0)
+	for (i = 0; i < items(command) && cmd == NULL; i++) {
+		if (strcmp(argv[2], command[i].str) != 0)
 			continue;
-		if (argc >= ops[i].args) {
-			cmd = ops[i].cmd;
+		if (argc >= command[i].args) {
+			cmd = command[i].cmd;
 			break;
 		}
-		err(EINVAL, "Too few args: command '%s'", ops[i].str);
+		err(EINVAL, "Too few args: command '%s'", command[i].str);
 	}
 }
 
 static void
 check_cmd_args(int argc, char *argv[])
 {
-	if ((cmd == NULL) && (argc > 2)) { /* nvm gbe [MAC] */
+	if (cmd == NULL && argc > 2) { /* nvm gbe [MAC] */
 		mac = argv[2];
 		cmd = cmd_setmac;
 	} else if (cmd == cmd_setmac) { /* nvm gbe setmac [MAC] */
 		mac = rmac; /* random MAC */
 		if (argc > 3)
 			mac = argv[3];
-	} else if ((cmd != NULL) && (argc > 3)) { /* user-supplied partnum */
+	} else if (cmd != NULL && argc > 3) { /* user-supplied partnum */
 		part = argv[3][0] - '0';
 		if (!((part == 0 || part == 1) && argv[3][1] == '\0'))
 			err(EINVAL, "Bad partnum: %s", argv[3]);
@@ -232,19 +302,27 @@ static void
 set_io_flags(int argc, char *argv[])
 {
 	flags = O_RDWR;
-	if (argc > 2) {
-		if (strcmp(argv[2], "dump") == 0)
-			flags = O_RDONLY;
-	}
+	if (argc < 3)
+		return;
+	if (strcmp(argv[2], "dump") == 0)
+		flags = O_RDONLY;
 }
 
 static void
 open_files(void)
 {
-	struct stat st;
+#ifndef HAVE_ARC4RANDOM
 	struct stat st_rfd;
-
-	xopen(&rfd, "/dev/urandom", O_RDONLY, &st_rfd);
+	rname = newrandom;
+	if ((rfd = open(rname, O_RDONLY)) == -1) {
+		/*
+		 * Fall back to /dev/random on old platforms
+		 * where /dev/urandom does not exist.
+		 */
+		rname = oldrandom;
+		xopen(&rfd, rname, O_RDONLY, &st_rfd);
+	}
+#endif
 	xopen(&fd, fname, flags, &st);
 
 	switch(st.st_size) {
@@ -274,15 +352,14 @@ read_gbe(void)
 	int p;
 	int do_read[2] = {1, 1};
 
-	if ((cmd == cmd_copy) || (cmd == cmd_brick) ||
-	    (cmd == cmd_setchecksum))
+	if (cmd == cmd_copy || cmd == cmd_brick || cmd == cmd_setchecksum)
 		do_read[part ^ 1] = 0;
 
 	/*
 	 * speedhack: if copy/swap, flip where data gets written to memory,
 	 * so that cmd_copy and cmd_swap don't have to work on every word
 	 */
-	if ((cmd == cmd_copy) || (cmd == cmd_swap))
+	if (cmd == cmd_copy || cmd == cmd_swap)
 		invert = 1;
 
 	for (p = 0; p < 2; p++) {
@@ -294,10 +371,8 @@ read_gbe(void)
 static void
 read_gbe_part(int p, int invert)
 {
-	read_file_PERFECTLY_or_die(fd, buf + (SIZE_4KB * (p ^ invert)),
-	    SIZE_4KB, ((off_t)p) * partsize, fname, "pread");
-
-	swap(p ^ invert);
+	read_file_PERFECTLY_or_die(fd, gbe_mem_offset(p ^ invert, "pwrite"),
+	    GBE_PART_SIZE, gbe_file_offset(p, "pread"), fname, "pread");
 }
 
 static void
@@ -320,7 +395,7 @@ parse_mac_string(void)
 {
 	int mac_pos;
 
-	if (strnlen(mac, 20) != 17)
+	if (strlen(mac) != 17)
 		err(EINVAL, "MAC address is the wrong length");
 
 	for (mac_pos = 0; mac_pos < 16; mac_pos += 3)
@@ -359,52 +434,70 @@ static void
 set_mac_nib(int mac_pos, int nib)
 {
 	uint8_t h;
-	int byte = mac_pos / 3;
+	int byte;
+	int shift;
 
 	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 */
-	if ((byte == 0) && (nib == 1)) {
-		if ((mac[mac_pos + nib] == '?') ||
-		    (mac[mac_pos + nib] == 'x') ||
-		    (mac[mac_pos + nib] == 'X')) /* random */
-			h = (h & 0xE) | 2; /* local, unicast */
-	}
+	byte = mac_pos / 3;
 
-	macbuf[byte >> 1] |= (uint16_t)h << ((8 * (byte % 2)) +
-	    (4 * (nib ^ 1)));
+	/* If random, ensure that local/unicast bits are set */
+	if ((byte == 0) && (nib == 1) &&
+	    ((mac[mac_pos + nib] == '?') ||
+	    (mac[mac_pos + nib] == 'x') ||
+	    (mac[mac_pos + nib] == 'X'))) /* random */
+		h = (h & 0xE) | 2; /* local, unicast */
+
+	/*
+	 * The word is stored big-endian in the file.
+	 * Logically in C, it is stored little-endian,
+	 * because of how we load in read_gbe(), so
+	 * we store the MAC address in reverse order
+	 * per 2-byte word (there are 3 of these).
+	 */
+	shift = (byte & 1) << 3; /* left or right byte? */
+	shift |= (nib ^ 1) << 2; /* left or right nib? */
+
+	/*
+	 * Now we can shift properly, OR'ing the result:
+	 */
+	macbuf[byte >> 1] |= (uint16_t)h << shift;
 }
 
 static uint8_t
 hextonum(char ch)
 {
-	if ((ch >= '0') && (ch <= '9'))
+	if ((unsigned)(ch - '0') <= 9)
 		return ch - '0';
-	else if ((ch >= 'A') && (ch <= 'F'))
-		return ch - 'A' + 10;
-	else if ((ch >= 'a') && (ch <= 'f'))
+
+	ch |= 0x20;
+
+	if ((unsigned)(ch - 'a') <= 5)
 		return ch - 'a' + 10;
-	else if ((ch == '?') || (ch == 'x') || (ch == 'X'))
-		return rhex(); /* random hex value */
+	else if (ch == '?' || ch == 'x')
+		return rhex(); /* random character */
 	else
-		return 16; /* error: invalid character */
+		return 16; /* invalid character */
 }
 
 static uint8_t
 rhex(void)
 {
-	static int n = -1;
 	static uint8_t rnum[12];
+	static size_t n = 0;
 
-	if (n == -1) {
-		n = sizeof(rnum) - 1;
-		read_file_PERFECTLY_or_die(rfd, rnum, sizeof(rnum),
-		    0, "/dev/urandom", NULL);
+	if (!n) {
+		n = sizeof(rnum);
+#ifdef HAVE_ARC4RANDOM
+		arc4random_buf(rnum, n);
+#else
+		read_file_PERFECTLY_or_die(rfd, rnum, n, 0, rname, NULL);
+#endif
 	}
 
-	return rnum[n--] & 0xf;
+	return rnum[--n] & 0xf;
 }
 
 static void
@@ -456,7 +549,7 @@ check_read_or_die(const char *rpath, ssize_t rval, size_t rsize,
 static int
 write_mac_part(int partnum)
 {
-	int w;
+	size_t w;
 
 	part = partnum;
 	if (!good_checksum(partnum))
@@ -487,14 +580,14 @@ cmd_dump(void)
 		hexdump(partnum);
 	}
 
-	if ((num_invalid < 2))
+	if (num_invalid < 2)
 		errno = 0;
 }
 
 static void
 print_mac_address(int partnum)
 {
-	int c;
+	size_t c;
 	for (c = 0; c < 3; c++) {
 		uint16_t val16 = word(c, partnum);
 		printf("%02x:%02x", val16 & 0xff, val16 >> 8);
@@ -508,12 +601,12 @@ print_mac_address(int partnum)
 static void
 hexdump(int partnum)
 {
-	int c;
-	int row;
+	size_t c;
+	size_t row;
 	uint16_t val16;
 
 	for (row = 0; row < 8; row++) {
-		printf("%08x ", row << 4);
+		printf("%08zx ", row << 4);
 		for (c = 0; c < 8; c++) {
 			val16 = word((row << 3) + c, partnum);
 			if (c == 4)
@@ -527,7 +620,7 @@ hexdump(int partnum)
 static void
 cmd_setchecksum(void)
 {
-	int c;
+	size_t c;
 	uint16_t val16 = 0;
 
 	for (c = 0; c < NVM_CHECKSUM_WORD; c++)
@@ -596,7 +689,7 @@ cmd_swap(void)
 static int
 good_checksum(int partnum)
 {
-	int w;
+	size_t w;
 	uint16_t total = 0;
 	for (w = 0; w <= NVM_CHECKSUM_WORD; w++)
 		total += word(w, partnum);
@@ -612,34 +705,37 @@ good_checksum(int partnum)
 }
 
 /*
- * 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.
+ * GbE NVM files store 16-bit (2-byte) little-endian words.
+ * We must therefore swap the order when reading or writing.
  */
 
 static uint16_t
-word(int pos16, int p)
+word(size_t pos16, int p)
 {
-	uint16_t rval = 0;
+	size_t pos;
 
 	check_bound(pos16, p);
-	memcpy(&rval, buf + (SIZE_4KB * p) + (pos16 << 1), sizeof(uint16_t));
+	pos = (pos16 << 1) + ((size_t)p * GBE_PART_SIZE);
 
-	return rval;
+	return (uint16_t)buf[pos] | ((uint16_t)buf[pos + 1] << 8);
 }
 
 static void
-set_word(int pos16, int p, uint16_t val16)
+set_word(size_t pos16, int p, uint16_t val16)
 {
+	size_t pos;
+
 	check_bound(pos16, p);
-	memcpy(buf + (SIZE_4KB * p) + (pos16 << 1), &val16, sizeof(uint16_t));
+	pos = (pos16 << 1) + ((size_t)p * GBE_PART_SIZE);
+
+	buf[pos] = (uint8_t)(val16 & 0xff);
+	buf[pos + 1] = (uint8_t)(val16 >> 8);
 
 	part_modified[p] = 1;
 }
 
 static void
-check_bound(int c, int p)
+check_bound(size_t c, int p)
 {
 	/*
 	 * NVM_SIZE assumed as the limit, because the
@@ -655,10 +751,10 @@ check_bound(int c, int p)
 	 * we ever wish to work on the extented area.
 	 */
 
-	if ((p != 0) && (p != 1))
+	if (p != 0 && p != 1)
 		err(EINVAL, "check_bound: invalid partnum %d", p);
-	if ((c < 0) || (c >= (NVM_SIZE >> 1)))
-		err(EINVAL, "check_bound: out of bounds %d", c);
+	if (c >= NVM_WORDS)
+		err(EINVAL, "check_bound: out of bounds %zu", c);
 }
 
 static void
@@ -678,49 +774,61 @@ write_gbe(void)
 static void
 write_gbe_part(int p)
 {
-	swap(p); /* swap bytes on big-endian host CPUs */
-
-	if (pwrite(fd, buf + (SIZE_4KB * p),
-	    SIZE_4KB, (off_t)p * partsize) != (ssize_t)SIZE_4KB) {
+	if (pwrite(fd, gbe_mem_offset(p, "pwrite"),
+	    GBE_PART_SIZE, gbe_file_offset(p, "pwrite")) != GBE_PART_SIZE) {
 		err(ECANCELED,
-		    "Can't write %d b to '%s' p%d", SIZE_4KB, fname, p);
+		    "Can't write %d b to '%s' p%d", GBE_PART_SIZE, fname, p);
 	}
 }
 
 /*
- * GbE files store bytes in little-endian order.
- * This function ensures big-endian host CPU support.
+ * Reads to GbE from write_gbe_part and read_gbe_part
+ * are filtered through here. These operations must
+ * only write from the 0th position or the half position
+ * within the GbE file, and write 4KB of data.
+ *
+ * This check is called, to ensure just that.
  */
-static void
-swap(int partnum)
+static off_t
+gbe_file_offset(int p, const char *f_op)
 {
-	/*
-	 * 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.
-	 */
+	return gbe_x_offset(p, f_op, "file",
+	    partsize, st.st_size);
+}
 
-	size_t w;
-	size_t x;
+/*
+ * This one is similar to gbe_file_offset,
+ * but used to check Gbe bounds in memory,
+ * and it is *also* used during file I/O.
+ */
+static void *
+gbe_mem_offset(int p, const char *f_op)
+{
+	off_t gbe_off = gbe_x_offset(p, f_op, "mem",
+	    GBE_PART_SIZE, GBE_FILE_SIZE);
 
-	uint8_t *n = buf + (SIZE_4KB * partnum);
+	return (void *)(buf + gbe_off);
+}
 
-	int e = 1;
-	if (*((uint8_t *)&e) == 1)
-		return; /* Little-endian host CPU; no swap needed. */
+static off_t
+gbe_x_offset(int p, const char *f_op, const char *d_type,
+    off_t nsize, off_t ncmp)
+{
+	off_t off = (off_t)p * nsize;
 
-	/*
-	 * The host CPU stores bytes in big-endian order.
-	 * We will therefore reverse the order in memory:
-	 */
-	for (w = 0, x = 1; w < NVM_SIZE; w += 2, x += 2) {
-		uint8_t chg = n[w];
-		n[w] = n[x];
-		n[x] = chg;
-	}
+	if ((unsigned int)p > 1)
+		err(ECANCELED, "GbE %s %s invalid partnum: %s",
+		    d_type, f_op, fname);
+
+	if (off + GBE_PART_SIZE > ncmp)
+		err(ECANCELED, "GbE %s %s out of bounds: %s",
+		    d_type, f_op, fname);
+
+	if (off != 0 && off != ncmp >> 1)
+		err(ECANCELED, "GbE %s %s at bad offset: %s",
+		    d_type, f_op, fname);
+
+	return off;
 }
 
 static void
@@ -770,10 +878,12 @@ getnvmprogname(void)
 {
 	const char *p;
 
-	if ((argv0 == NULL) || (*argv0 == '\0'))
+	if (argv0 == NULL || *argv0 == '\0')
 		return "";
 
-	if ((p = strrchr(argv0, '/')))
+	p = strrchr(argv0, '/');
+
+	if (p)
 		return p + 1;
 	else
 		return argv0;