diff options
Diffstat (limited to 'util/nvmutil')
| -rw-r--r-- | util/nvmutil/Makefile | 12 | ||||
| -rw-r--r-- | util/nvmutil/nvmutil.c | 409 |
2 files changed, 288 insertions, 133 deletions
diff --git a/util/nvmutil/Makefile b/util/nvmutil/Makefile index bef6f28c..719e1c1e 100644 --- a/util/nvmutil/Makefile +++ b/util/nvmutil/Makefile @@ -1,9 +1,10 @@ # SPDX-License-Identifier: MIT -# SPDX-FileCopyrightText: 2022,2026 Leah Rowe <leah@libreboot.org> -# SPDX-FileCopyrightText: 2023 Riku Viitanen <riku.viitanen@protonmail.com> +# Copyright (c) 2022,2026 Leah Rowe <leah@libreboot.org> +# Copyright (c) 2023 Riku Viitanen <riku.viitanen@protonmail.com> CC?=cc CFLAGS?=-Os -Wall -Wextra -Werror -pedantic -std=c90 +LDFLAGS?= DESTDIR?= PREFIX?=/usr/local INSTALL?=install @@ -13,11 +14,12 @@ PROG=nvmutil all: $(PROG) $(PROG): nvmutil.c - $(CC) $(CFLAGS) nvmutil.c -o $(PROG) + $(CC) $(CFLAGS) $(LDFLAGS) nvmutil.c -o $(PROG) install: $(PROG) - mkdir -p $(DESTDIR)$(PREFIX)/bin/ - install $(PROG) $(DESTDIR)$(PREFIX)/bin/ + $(INSTALL) -d $(DESTDIR)$(PREFIX)/bin + $(INSTALL) $(PROG) $(DESTDIR)$(PREFIX)/bin/$(PROG) + chmod 755 $(DESTDIR)$(PREFIX)/bin/$(PROG) uninstall: rm -f $(DESTDIR)$(PREFIX)/bin/$(PROG) diff --git a/util/nvmutil/nvmutil.c b/util/nvmutil/nvmutil.c index 68e041a3..da6336aa 100644 --- a/util/nvmutil/nvmutil.c +++ b/util/nvmutil/nvmutil.c @@ -12,9 +12,131 @@ * * Recommended CFLAGS for Clang/GCC: * - * -Os -Wall -Wextra -Werror -pedantic -std=c99 + * -Os -Wall -Wextra -Werror -pedantic -std=c90 */ +/* + * Major TODO: split this into multiple files. + * This program has become quite large now, mostly + * due to all the extra sanity checks / portability. + * Make most of nvmutil a *library* for re-use + * + * TODO: gettimeofday not posible - use portable functions. + * TODO: uint32_t fallback: modify the program instead + * to run on 16-bit systems: smaller buffers, and do + * operations byte-based instead of word-based. + * + * TODO: _XOPEN_SOURCE 500 probably not needed anymore. + * the portable fallbacks alone are likely enough. + * e.g. i don't need stdint, and i don't use pwrite/pread + * anymore. + * + * TODO: version detection of various BSDs to detect + * arc4random, use that if available. but also work on + * older versions of those BSDs (also MacOS) that lack it. + * + * TODO: portability/testing on non-Unix systems: + * old DOS. all windows versions (probably irrelevant + * because you can use cygwin/wsl, whatever), classic MacOS, + * also test really old unix e.g. sunos and irix. Be/Haiku too! + * + * TODO: reliance on global variables for status. make + * functions use structs passed as args instead, make + * functions re-useable (including libraries), etc. + * + * TODO: bound checks for files per-command, e.g. only + * first 6 bytes for CMD_SETMAC + * + * TODO: in command sanitizer: verify that each given + * entry corresponds to the correct function, in the + * pointer (this check is currently missing) + * + * TODO: general modularisierung of the entire codebase. + * TODO: better explain copy/swap read inversion trick + * by improving existing comments + * TODO: lots of overwritten comments in code. tidy it up. + * + * TODO: use getopt for nvmutil args, so that multiple + * operations can be performed, and also on many + * files at once (noting limitations with cat) + * BONUS: implement own getopt(), for portability + * + * TODO: document fuzzing / static analysis methods + * for the code, and: + * TODO: implement rigorous unit tests (separate util) + * NOTE: this would *include* known good test files + * in various configurations, also invalid files. + * the tests would likely be portable posix shell + * scripts rather than a new C program, but a modularisiert + * codebase would allow me to write a separate C + * program to test some finer intricacies + * TODO: the unit tests would basically test regressions + * TODO: after writing back a gbe to file, close() and + * open() it again, read it again, and check that + * the contents were written correctly, providing + * a warning if they were. do this in the main + * program. + * TODO: the unit tests would include an aggressive set + * of fuzz tests, under controlled conditions + * + * TODO: also document the layout of Intel GbE files, so + * that wily individuals can easily expand the + * featureset of nvmutil. + * TODO: write a manpage + * TODO: simplify the command sanitization, implement more + * of it as build time checks, e.g. static asserts. + * generally remove cleverness from the code, instead + * prefyerring readibility + * TODO: also document nvmutil's coding style, which is + * its own style at this point! + * TODO: when all the above (and possibly more) is done, + * submit this tool to coreboot with a further change + * to their build system that lets users modify + * GbE images, especially set MAC addresses, when + * including GbE files in coreboot configs. + */ +/* + BONUS TODO: + CI/CD. woodpecker is good enough, sourcehut also has one. + tie this in with other things mentioned here, + e.g. fuzzer / unit tests +*/ + +/* Major TODO: reproducible builds +Test with and without these: + +CFLAGS += -fno-record-gcc-switches +CFLAGS += -ffile-prefix-map=$(PWD)=. +CFLAGS += -fdebug-prefix-map=$(PWD)=. + +I already avoid unique timestamps per-build, +by not using them, e.g. not reporting build +time in the program. + +When splitting the nvmutil.c file later, do e.g.: + +SRC = main.c io.c nvm.c cmd.c +OBJ = $(SRC:.c=.o) + +^ explicitly declare the order in which to build +*/ + +/* +TODO: +further note when fuzzing is implemented: +use deterministic randomisation, with a +guaranteed seed - so e.g. don't use /dev/urandom +in test builds. e.g. just use normal rand() +but with a static seed e.g. 1234 +*/ +/* +TODO: stricter build flags, e.g. +CFLAGS += -fstack-protector-strong +CFLAGS += -fno-common +CFLAGS += -D_FORTIFY_SOURCE=2 +CFLAGS += -fPIE +*/ + #ifndef _XOPEN_SOURCE #define _XOPEN_SOURCE 500 #endif @@ -27,6 +149,7 @@ #include <sys/param.h> #endif #include <sys/types.h> +#include <sys/time.h> #include <sys/stat.h> #include <errno.h> @@ -51,6 +174,7 @@ typedef unsigned int uint32_t; #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <time.h> #include <unistd.h> typedef char static_assert_char_is_8_bits[(CHAR_BIT == 8) ? 1 : -1]; @@ -161,6 +285,8 @@ static void set_mac_nib(size_t mac_str_pos, size_t mac_byte_pos, size_t mac_nib_pos); static uint16_t hextonum(char ch_s); static uint16_t rhex(void); +static uint16_t fallback_rand(void); +static unsigned long entropy_jitter(void); static void write_mac_part(size_t partnum); /* @@ -211,6 +337,8 @@ static off_t gbe_x_offset(size_t part, const char *f_op, const char *d_type, off_t nsize, off_t ncmp); static ssize_t rw_file_exact(int fd, uint8_t *mem, size_t len, off_t off, int rw_type); +static ssize_t rw_file_once(int fd, uint8_t *mem, size_t len, + off_t off, int rw_type, size_t rc); static ssize_t do_rw(int fd, uint8_t *mem, size_t len, off_t off, int rw_type); static ssize_t prw(int fd, void *mem, size_t nrw, @@ -223,7 +351,6 @@ static off_t lseek_eintr(int fd, off_t off, int whence); static void err(int nvm_errval, const char *msg, ...); static void close_files(void); static const char *getnvmprogname(void); -static void set_err_if_unset(int errval); static void usage(uint8_t usage_exit); /* @@ -261,7 +388,6 @@ static void usage(uint8_t usage_exit); #define items(x) (sizeof((x)) / sizeof((x)[0])) static const char newrandom[] = "/dev/urandom"; -static const char oldrandom[] = "/dev/random"; /* fallback on OLD unix */ static const char *rname = NULL; /* @@ -308,10 +434,10 @@ static const char *argv0; #define ARGC_4 4 enum { - LESEN, - PLESEN, - SCHREIB, - PSCHREIB + IO_READ, + IO_WRITE, + IO_PREAD, + IO_PWRITE }; /* @@ -434,6 +560,12 @@ static size_t cmd_index = CMD_NULL; typedef char assert_argc3[(ARGC_3==3)?1:-1]; typedef char assert_argc4[(ARGC_4==4)?1:-1]; +typedef char assert_read[(IO_READ==0)?1:-1]; +typedef char assert_write[(IO_WRITE==1)?1:-1]; +typedef char assert_pread[(IO_PREAD==2)?1:-1]; +typedef char assert_pwrite[(IO_PWRITE==3)?1:-1]; + +static int use_prng = 0; int main(int argc, char *argv[]) @@ -448,8 +580,8 @@ main(int argc, char *argv[]) #ifdef NVMUTIL_UNVEIL if (pledge("stdio rpath wpath unveil", NULL) == -1) err(errno, "pledge"); - if (unveil("/dev/null", "r") == -1) - err(errno, "unveil '/dev/null'"); + if (unveil("/dev/urandom", "r") == -1) + err(errno, "unveil /dev/urandom"); #else if (pledge("stdio rpath wpath", NULL) == -1) err(errno, "pledge"); @@ -503,20 +635,13 @@ main(int argc, char *argv[]) read_gbe_file(); read_checksums(); - errno = 0; run_cmd(cmd_index); - if (errno && (!(part_valid[0] || part_valid[1]))) - err(errno, "%s: Unhandled error (WRITE SKIPPED)", fname); - if (command[cmd_index].flags == O_RDWR) write_gbe_file(); close_files(); - if (errno) - err(errno, "Unhandled error on exit"); - return EXIT_SUCCESS; } @@ -608,9 +733,6 @@ sanitize_command_index(size_t c) if (command[c].flags != O_RDONLY && command[c].flags != O_RDWR) err(EINVAL, "invalid cmd.flags setting"); - - if (!((!LESEN) && (PLESEN == 1) && (SCHREIB == 2) && (PSCHREIB == 3))) - err(EINVAL, "rw type integers are the wrong values"); } static void @@ -714,7 +836,7 @@ xstrxcmp(const char *a, const char *b, size_t maxlen) /* * Should never reach here. This keeps compilers happy. */ - set_err_if_unset(EINVAL); + errno = EINVAL; return -1; } @@ -722,26 +844,13 @@ static void open_dev_urandom(void) { rname = newrandom; - urandom_fd = open(rname, O_RDONLY | O_BINARY | O_NONBLOCK); + urandom_fd = open(rname, O_RDONLY); if (urandom_fd != -1) return; - /* - * Fall back to /dev/random on very old Unix. - * - * We must reset errno, to remove stale state - * set by reading /dev/urandom - */ - - fprintf(stderr, "Can't open %s (will use %s instead)\n", - newrandom, oldrandom); - - errno = 0; - - rname = oldrandom; - urandom_fd = open(rname, O_RDONLY | O_BINARY | O_NONBLOCK); - if (urandom_fd == -1) - err(errno, "%s: could not open", rname); + /* fallback on VERY VERY VERY old unix */ + use_prng = 1; + srand((unsigned)(time(NULL) ^ getpid())); } static void @@ -791,7 +900,7 @@ read_gbe_file(void) for (p = 0; p < 2; p++) { if (do_read[p]) - rw_gbe_file_part(p, PLESEN, "pread"); + rw_gbe_file_part(p, IO_PREAD, "pread"); } } @@ -837,14 +946,11 @@ read_checksums(void) ++num_invalid; } - if (num_invalid < max_invalid) - errno = 0; - if (num_invalid >= max_invalid) { if (max_invalid == 1) - err(EINVAL, "%s: part %lu has a bad checksum", + err(ECANCELED, "%s: part %lu has a bad checksum", fname, (unsigned long)part); - err(EINVAL, "%s: No valid checksum found in file", + err(ECANCELED, "%s: No valid checksum found in file", fname); } } @@ -858,7 +964,6 @@ good_checksum(size_t partnum) if (current_checksum == expected_checksum) return 1; - set_err_if_unset(EINVAL); return 0; } @@ -874,7 +979,7 @@ static void check_command_num(size_t c) { if (!valid_command(c)) - err(errno, "Invalid run_cmd arg: %lu", + err(EINVAL, "Invalid run_cmd arg: %lu", (unsigned long)c); } @@ -1022,16 +1127,64 @@ rhex(void) static size_t n = 0; static uint8_t rnum[12]; + if (use_prng) + return fallback_rand(); + if (!n) { n = sizeof(rnum); - if (rw_file_exact(urandom_fd, rnum, n, 0, LESEN) == -1) + if (rw_file_exact(urandom_fd, rnum, n, 0, IO_READ) == -1) err(errno, "Randomisation failed"); - errno = 0; } return (uint16_t)(rnum[--n] & 0xf); } +static uint16_t +fallback_rand(void) +{ + struct timeval tv; + unsigned long mix; + static unsigned long counter = 0; + + gettimeofday(&tv, NULL); + + mix = (unsigned long)tv.tv_sec + ^ (unsigned long)tv.tv_usec + ^ (unsigned long)getpid() + ^ (unsigned long)&mix + ^ counter++ + ^ entropy_jitter(); + + /* + * Stack addresses can vary between + * calls, thus increasing entropy. + */ + mix ^= (unsigned long)&mix; + mix ^= (unsigned long)&tv; + mix ^= (unsigned long)&counter; + + return (uint16_t)(mix & 0xf); +} + +static unsigned long +entropy_jitter(void) +{ + struct timeval a, b; + unsigned long mix = 0; + int i; + + for (i = 0; i < 8; i++) { + gettimeofday(&a, NULL); + getpid(); + gettimeofday(&b, NULL); + + mix ^= (unsigned long)(b.tv_usec - a.tv_usec); + mix ^= (unsigned long)&mix; + } + + return mix; +} + static void write_mac_part(size_t partnum) { @@ -1057,9 +1210,6 @@ cmd_helper_dump(void) part_valid[0] = good_checksum(0); part_valid[1] = good_checksum(1); - if (part_valid[0] || part_valid[1]) - errno = 0; - for (partnum = 0; partnum < 2; partnum++) { if (!part_valid[partnum]) fprintf(stderr, @@ -1079,9 +1229,10 @@ static void print_mac_from_nvm(size_t partnum) { size_t c; + uint16_t val16; for (c = 0; c < 3; c++) { - uint16_t val16 = nvm_word(c, partnum); + val16 = nvm_word(c, partnum); printf("%02x:%02x", val16 & 0xff, val16 >> 8); if (c == 2) printf("\n"); @@ -1140,7 +1291,7 @@ gbe_cat_buf(uint8_t *b) while (1) { rval = rw_file_exact(STDOUT_FILENO, b, - GBE_PART_SIZE, 0, SCHREIB); + GBE_PART_SIZE, 0, IO_WRITE); if (rval >= 0) { /* @@ -1155,19 +1306,7 @@ gbe_cat_buf(uint8_t *b) if (errno != EAGAIN) err(errno, "stdout: cat"); - - /* - * We assume that no data - * was written to stdout. - */ - errno = 0; } - - /* - * No errors here. - * Avoid the warning in main() - */ - errno = 0; } static void @@ -1193,7 +1332,7 @@ write_gbe_file(void) if (update_checksum) set_checksum(partnum); - rw_gbe_file_part(partnum, PSCHREIB, "pwrite"); + rw_gbe_file_part(partnum, IO_PWRITE, "pwrite"); } } @@ -1317,7 +1456,7 @@ rw_gbe_file_part(size_t p, int rw_type, uint8_t *mem_offset; - if (rw_type == SCHREIB || rw_type == PSCHREIB) + if (rw_type == IO_WRITE || rw_type == IO_PWRITE) invert = 0; /* @@ -1331,8 +1470,6 @@ rw_gbe_file_part(size_t p, int rw_type, rw_type) == -1) err(errno, "%s: %s: part %lu", fname, rw_type_str, (unsigned long)p); - - errno = 0; } /* @@ -1402,54 +1539,94 @@ gbe_x_offset(size_t p, const char *f_op, const char *d_type, * be used on sockets or pipes, because 0-byte * reads are treated like fatal errors. This * means that EOF is also considered fatal. + * + * WARNING: Do not use O_APPEND on open() when + * using this function. If you do, POSIX allows + * write() to ignore the current file offset and + * write at EOF, which means that our use of + * lseek in prw() does not guarantee writing at + * a specified offset. So if using IO_PWRITE or + * IO_PREAD, make sure not to pass a file descriptor + * with the O_APPEND flag. Alternatively, modify + * do_rw() to directly use pwrite() and pread() + * instead of prw(). */ static ssize_t rw_file_exact(int fd, uint8_t *mem, size_t len, off_t off, int rw_type) { - ssize_t rval = 0; - size_t rc = 0; + ssize_t rv; + size_t rc; - if (fd < 0 || !len || len > (size_t)SSIZE_MAX) { - set_err_if_unset(EIO); + if (fd < 0 || !len || len > (size_t)SSIZE_MAX + || (unsigned int)rw_type > IO_PWRITE) { + errno = EIO; return -1; } - while (rc < len) { - rval = do_rw(fd, mem + rc, len - rc, off + rc, rw_type); - - if (rval < 0 && errno == EINTR) { - continue; - } else if (rval < 0) { - set_err_if_unset(EIO); + for (rc = 0, rv = 0; rc < len; ) { + if ((rv = rw_file_once(fd, mem, len, off, rw_type, rc)) <= 0) return -1; - } - if ((size_t)rval > (len - rc) /* Prevent overflow */ - || rval == 0) { /* Prevent infinite 0-byte loop */ - set_err_if_unset(EIO); - return -1; - } - rc += (size_t)rval; + rc += (size_t)rv; } return rc; } +/* + * May not return all requested bytes (len). + * Use rw_file_exact for guaranteed length. + */ +static ssize_t +rw_file_once(int fd, uint8_t *mem, size_t len, + off_t off, int rw_type, size_t rc) +{ + ssize_t rv; + size_t retries_on_zero = 0; + size_t max_retries = 10; + +read_again: + if ((unsigned int)rw_type > IO_PWRITE) + goto err_rw_file_once; + + rv = do_rw(fd, mem + rc, len - rc, off + rc, rw_type); + + if (rv < 0 && errno == EINTR) + goto read_again; + + if (rv < 0) + return -1; + + if ((size_t)rv > SSIZE_MAX /* theoretical buggy libc */ + || (size_t)rv > (len - rc))/* don't overflow */ + goto err_rw_file_once; + + if (rv != 0) + return rv; + + if (retries_on_zero++ < max_retries) + goto read_again; + +err_rw_file_once: + errno = EIO; + return -1; +} + static ssize_t do_rw(int fd, uint8_t *mem, size_t len, off_t off, int rw_type) { - if (rw_type == LESEN || rw_type == PLESEN << 2) + if (rw_type == IO_READ) return read(fd, mem, len); - if (rw_type == SCHREIB || rw_type == PSCHREIB << 2) + if (rw_type == IO_WRITE) return write(fd, mem, len); - if (rw_type == PLESEN || rw_type == PSCHREIB) + if (rw_type == IO_PREAD || rw_type == IO_PWRITE) return prw(fd, mem, len, off, rw_type); - set_err_if_unset(EINVAL); + errno = EIO; return -1; } @@ -1469,6 +1646,14 @@ prw(int fd, void *mem, size_t nrw, off_t off_orig; ssize_t r; int saved_errno; + int prw_type; + + prw_type = rw_type ^ IO_PREAD; + + if ((unsigned int)prw_type > IO_WRITE) { + errno = EIO; + return -1; + } if ((off_orig = lseek_eintr(fd, (off_t)0, SEEK_CUR)) == (off_t)-1) return -1; @@ -1476,7 +1661,7 @@ prw(int fd, void *mem, size_t nrw, return -1; do { - r = do_rw(fd, mem, nrw, off, rw_type << 2); + r = do_rw(fd, mem, nrw, off, prw_type); } while (r < 0 && errno == EINTR); saved_errno = errno; @@ -1507,23 +1692,12 @@ err(int nvm_errval, const char *msg, ...) { va_list args; - /* - * We need to ensure that files are closed - * on exit, including error exits. This - * would otherwise recurse, because the - * close_files() function also calls err(), - * but with -1 on nvm_errval. It's the only - * one that does this. - * - * Since the errval is for setting errno, -1 - * would be incorrect. Therefore, set_err_if_unset() - * avoids overriding errno if the given value - * is negative. - * - * Be careful modifying err() and close_files(). - */ - if (nvm_errval != -1) + if (nvm_errval >= 0) { close_files(); + errno = nvm_errval; + } + if (errno <= 0) + errno = ECANCELED; fprintf(stderr, "%s: ", getnvmprogname()); @@ -1531,7 +1705,6 @@ err(int nvm_errval, const char *msg, ...) vfprintf(stderr, msg, args); va_end(args); - set_err_if_unset(nvm_errval); fprintf(stderr, ": %s", strerror(errno)); fprintf(stderr, "\n"); @@ -1570,26 +1743,6 @@ getnvmprogname(void) return argv0; } -/* - * Set errno only if it hasn't already been set. - * This prevents overriding real libc errors. - * - * We use errno for regular program state, while - * being careful not to clobber what was set by - * real libc function, or a minority of our stub - * functions such as prw() - */ -static void -set_err_if_unset(int x) -{ - if (errno) - return; - if (x > 0) - errno = x; - else - errno = ECANCELED; -} - static void usage(uint8_t usage_exit) { |