diff options
| author | Leah Rowe <leah@libreboot.org> | 2026-03-18 19:30:32 +0000 |
|---|---|---|
| committer | Leah Rowe <leah@libreboot.org> | 2026-03-19 04:25:43 +0000 |
| commit | 2ed8db3adc19dd922e31082634146e159f65af2e (patch) | |
| tree | dd242e1f7a178ee0c925cd080d61bac22fd681f5 /util | |
| parent | 6ccd54635fdec85f44a9960b93c52fde89c07f41 (diff) | |
util/nvmutil: major cleanup
handle init in xstatus()
it's now a singleton design
also tidied up some other code
also removed todo.c. bloat.
will do all those anyway.
too much change. i just kept
touching the code until it
looked good
Signed-off-by: Leah Rowe <leah@libreboot.org>
Diffstat (limited to 'util')
| -rw-r--r-- | util/nvmutil/include/common.h | 232 | ||||
| -rw-r--r-- | util/nvmutil/lib/checksum.c | 9 | ||||
| -rw-r--r-- | util/nvmutil/lib/command.c | 139 | ||||
| -rw-r--r-- | util/nvmutil/lib/file.c | 127 | ||||
| -rw-r--r-- | util/nvmutil/lib/io.c | 201 | ||||
| -rw-r--r-- | util/nvmutil/lib/num.c | 4 | ||||
| -rw-r--r-- | util/nvmutil/lib/state.c | 192 | ||||
| -rw-r--r-- | util/nvmutil/lib/string.c | 55 | ||||
| -rw-r--r-- | util/nvmutil/lib/word.c | 26 | ||||
| -rw-r--r-- | util/nvmutil/nvmutil.c | 117 | ||||
| -rw-r--r-- | util/nvmutil/todo.c | 134 |
11 files changed, 427 insertions, 809 deletions
diff --git a/util/nvmutil/include/common.h b/util/nvmutil/include/common.h index 18f74412..f9a4ba1c 100644 --- a/util/nvmutil/include/common.h +++ b/util/nvmutil/include/common.h @@ -4,26 +4,33 @@ * Copyright (c) 2022-2026 Leah Rowe <leah@libreboot.org> */ -/* - * TODO: split this per .c file - */ - -/* Use this shorthand in cmd helpers. e.g. - in cmd_setmac function: - check_cmd(cmd_helper_cat); -*/ - #ifndef COMMON_H #define COMMON_H -/* - * system prototypes +/* keep SYS_RENAME 1 to + * use libc rename() + * recommended + */ +#ifndef SYS_RENAME +#define SYS_RENAME 1 +#endif + +#define items(x) (sizeof((x)) / sizeof((x)[0])) + +/* system prototypes */ int fchmod(int fd, mode_t mode); -/* - * build config +/* analog of SSIZE_MAX + */ + +#ifndef X_LONG_MAX +#define X_LONG_MAX ((long)(~((long)1 << (sizeof(long)*CHAR_BIT-1)))) +#endif + + +/* build config */ #ifndef NVMUTIL_H @@ -71,27 +78,6 @@ int fchmod(int fd, mode_t mode); #define _FILE_OFFSET_BITS 64 #endif -/* - * Older versions of BSD to the early 2000s - * could compile nvmutil, but pledge was - * added in the 2010s. Therefore, for extra - * portability, we will only pledge/unveil - * on OpenBSD versions that have it. - */ - -#if defined(__OpenBSD__) && defined(OpenBSD) -#if OpenBSD >= 604 -#ifndef NVMUTIL_UNVEIL -#define NVMUTIL_UNVEIL 1 -#endif -#endif -#if OpenBSD >= 509 -#ifndef NVMUTIL_PLEDGE -#define NVMUTIL_PLEDGE 1 -#endif -#endif -#endif - #ifndef EXIT_FAILURE #define EXIT_FAILURE 1 #endif @@ -132,8 +118,7 @@ int fchmod(int fd, mode_t mode); #define FD_CLOEXEC 0 #endif -/* - * Sizes in bytes: +/* Sizes in bytes: */ #define SIZE_1KB 1024 @@ -144,17 +129,12 @@ int fchmod(int fd, mode_t mode); #define GBE_BUF_SIZE (SIZE_128KB) -/* - * 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. +/* First 128 bytes of gbe.bin is NVM. + * Then extended area. All of NVM must + * add up to BABA, truncated (LE) * - * There is a second 4KB part with the same - * rules, and it *should* be identical. + * First 4KB of each half of the file + * contains NVM+extended. */ #define GBE_WORK_SIZE (SIZE_8KB) @@ -164,29 +144,7 @@ int fchmod(int fd, mode_t mode); #define NVM_WORDS (NVM_SIZE >> 1) #define NVM_CHECKSUM_WORD (NVM_WORDS - 1) -/* - * Portable macro based on BSD nitems. - * Used to count the number of commands (see below). - */ - -#define items(x) (sizeof((x)) / sizeof((x)[0])) - -/* - * GbE files can be 8KB, 16KB or 128KB, - * but we only need the two 4KB parts - * from offset zero and offset 64KB in - * a 128KB file, or zero and 8KB in a 16KB - * file, or zero and 4KB in an 8KB file. - * - * The code will handle this properly. - */ - -#ifndef X_LONG_MAX -#define X_LONG_MAX ((long)(~((long)1 << (sizeof(long)*CHAR_BIT-1)))) -#endif - -/* - * Use these for .argc in command[]: +/* argc minimum (dispatch) */ #define ARGC_3 3 @@ -195,16 +153,11 @@ int fchmod(int fd, mode_t mode); #define NO_LOOP_EAGAIN 0 #define NO_LOOP_EINTR 0 -/* - * Used for checking whether. - * a file is a file via stat(). - * - * Portable macro for compatibility - * with older unix e.g. v7 unix (has S_IFREG), - * 4.2bsd (has S_IFMT) or POSIX (has S_ISREG) +/* For checking if an fd is a normal file. + * Portable for old Unix e.g. v7 (S_IFREG), + * 4.2BSD (S_IFMT), POSIX (S_ISREG). * - * Fallback works where S_IFREG == 0100000 - * (classic unix bitmask) + * IFREG: assumed 0100000 (classic bitmask) */ #ifndef S_ISREG @@ -222,9 +175,7 @@ int fchmod(int fd, mode_t mode); #define IO_PREAD 2 #define IO_PWRITE 3 -/* - * Used as indices for command[] - * MUST be in the same order as entries in command[] +/* for nvmutil commands */ #define CMD_DUMP 0 @@ -244,6 +195,9 @@ int fchmod(int fd, mode_t mode); #define SKIP_CHECKSUM_WRITE 0 #define CHECKSUM_WRITE 1 +/* command table + */ + struct commands { unsigned long chk; char *str; @@ -256,15 +210,24 @@ struct commands { int flags; /* e.g. O_RDWR or O_RDONLY */ }; +/* mac address + */ + struct macaddr { char *str; /* set to rmac, or argv string */ char rmac[18]; /* xx:xx:xx:xx:xx:xx */ unsigned short mac_buf[3]; }; +/* gbe.bin and tmpfile + */ + struct xfile { int gbe_fd; + struct stat gbe_st; + int tmp_fd; + struct stat tmp_st; char *tname; /* path of tmp file */ char *fname; /* path of gbe file */ @@ -279,12 +242,6 @@ struct xfile { int post_rw_checksum[2]; - dev_t gbe_dev; - ino_t gbe_ino; - - dev_t tmp_dev; - ino_t tmp_ino; - off_t gbe_file_size; off_t gbe_tmp_size; @@ -298,11 +255,13 @@ struct xfile { unsigned char pad[GBE_WORK_SIZE]; /* the file that wouldn't die */ }; -/* +/* Command table, MAC address, files + * * BE CAREFUL when editing this * to ensure that you also update * the tables in xstatus() */ + struct xstate { struct commands cmd[7]; struct macaddr mac; @@ -313,10 +272,6 @@ struct xstate { unsigned long i; /* index to cmd[] for current command */ int no_cmd; - /* store size of a struct here. - (can be used to copy old state) */ - unsigned long xsize; - /* Cat commands set this. the cat cmd helpers check it */ int cat; @@ -324,52 +279,47 @@ struct xstate { +struct xstate *xstatus(int argc, char *argv[]); -struct xstate *xstatus(void); - -/* - * Sanitize command tables. +/* Sanitize command tables. */ + void sanitize_command_list(void); void sanitize_command_index(unsigned long c); -/* - * Argument handling (user input) +/* Argument handling (user input) */ + void set_cmd(int argc, char *argv[]); void set_cmd_args(int argc, char *argv[]); unsigned long conv_argv_part_num(const char *part_str); int xstrxcmp(const char *a, const char *b, unsigned long maxlen); -/* - * Prep files for reading +/* Prep files for reading */ + void open_gbe_file(void); int lock_file(int fd, int flags); +int same_file(int fd, struct stat *st_old, int check_size); void xopen(int *fd, const char *path, int flags, struct stat *st); -/* - * Read GbE file and verify - * checksums. - * - * After this, we can run commands. +/* Read GbE file and verify checksums */ + void copy_gbe(void); void read_file(void); void read_checksums(void); int good_checksum(unsigned long partnum); -/* - * Execute user command on GbE data. - * These are stubs that call helpers. +/* validate commands */ -void run_cmd(void); + void check_command_num(unsigned long c); unsigned char valid_command(unsigned long c); -/* - * Helper functions for command: setmac +/* Helper functions for command: setmac */ + void cmd_helper_setmac(void); void parse_mac_string(void); unsigned long xstrxlen(const char *scmp, unsigned long maxlen); @@ -380,51 +330,49 @@ unsigned short hextonum(char ch_s); unsigned long rlong(void); void write_mac_part(unsigned long partnum); -/* - * Helper functions for command: dump +/* Helper functions for command: dump */ + void cmd_helper_dump(void); void print_mac_from_nvm(unsigned long partnum); void hexdump(unsigned long partnum); -/* - * Helper functions for command: swap +/* Helper functions for command: swap */ + void cmd_helper_swap(void); -/* - * Helper functions for command: copy +/* Helper functions for command: copy */ + void cmd_helper_copy(void); -/* - * Helper functions for commands: +/* Helper functions for commands: * cat, cat16 and cat128 */ + void cmd_helper_cat(void); void cmd_helper_cat16(void); void cmd_helper_cat128(void); void cat(unsigned long nff); void cat_buf(unsigned char *b); +/* Command verification/control + */ + void check_cmd(void (*fn)(void), const char *name); void cmd_helper_err(void); -/* - * After command processing, write - * the modified GbE file back. - * - * These are stub functions: check - * below for the actual functions. +/* Write GbE files to disk */ + void write_gbe_file(void); void set_checksum(unsigned long part); unsigned short calculated_checksum(unsigned long p); -/* - * Helper functions for accessing - * the NVM area during operation. +/* NVM read/write */ + unsigned short nvm_word(unsigned long pos16, unsigned long part); void set_nvm_word(unsigned long pos16, unsigned long part, unsigned short val16); @@ -432,23 +380,26 @@ void set_part_modified(unsigned long p); void check_nvm_bound(unsigned long pos16, unsigned long part); void check_bin(unsigned long a, const char *a_name); -/* - * Helper functions for stub functions - * that handle GbE file reads/writes. +/* GbE file read/write */ + void rw_gbe_file_part(unsigned long p, int rw_type, const char *rw_type_str); void write_to_gbe_bin(void); int gbe_mv(void); void check_written_part(unsigned long p); void report_io_err_rw(void); -int fsync_dir(const char *path); unsigned char *gbe_mem_offset(unsigned long part, const char *f_op); off_t gbe_file_offset(unsigned long part, const char *f_op); off_t gbe_x_offset(unsigned long part, const char *f_op, const char *d_type, off_t nsize, off_t ncmp); long rw_gbe_file_exact(int fd, unsigned char *mem, unsigned long nrw, off_t off, int rw_type); + +/* Generic read/write + */ + +int fsync_dir(const char *path); long rw_file_exact(int fd, unsigned char *mem, unsigned long len, off_t off, int rw_type, int loop_eagain, int loop_eintr, unsigned long max_retries, int off_reset); @@ -466,27 +417,21 @@ off_t lseek_loop(int fd, off_t off, #endif int try_err(int loop_err, int errval); -/* - * Error handling and cleanup +/* Error handling and cleanup */ + void usage(void); void err(int nvm_errval, const char *msg, ...); int exit_cleanup(void); const char *getnvmprogname(void); -/* - * a special kind of hell +/* Portable libc functions */ + char *new_tmpfile(int *fd, int local, const char *path); int x_i_mkstemp(char *template); char *x_c_strrchr(const char *s, int c); -/* x_i_rename not suitable - * for atomic writes. kept - * commentted for use in a - * library in the future */ -/* int x_i_rename(const char *src, const char *dst); -*/ char *x_c_tmpdir(void); int x_i_close(int fd); void *x_v_memcpy(void *dst, @@ -495,9 +440,6 @@ int x_i_memcmp(const void *a, const void *b, unsigned long n); int x_i_fsync(int fd); - - - /* asserts */ /* type asserts */ diff --git a/util/nvmutil/lib/checksum.c b/util/nvmutil/lib/checksum.c index 35b88eb9..d006a106 100644 --- a/util/nvmutil/lib/checksum.c +++ b/util/nvmutil/lib/checksum.c @@ -29,9 +29,9 @@ void read_checksums(void) { - struct xstate *x = xstatus(); - struct commands *cmd; - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct commands *cmd = &x->cmd[x->i]; + struct xfile *f = &x->f; unsigned long _p; unsigned long _skip_part; @@ -39,9 +39,6 @@ read_checksums(void) unsigned char _num_invalid; unsigned char _max_invalid; - cmd = &x->cmd[x->i]; - f = &x->f; - f->part_valid[0] = 0; f->part_valid[1] = 0; diff --git a/util/nvmutil/lib/command.c b/util/nvmutil/lib/command.c index 05ac8bbb..b70430aa 100644 --- a/util/nvmutil/lib/command.c +++ b/util/nvmutil/lib/command.c @@ -24,13 +24,13 @@ #include "../include/common.h" -/* - * Guard against regressions by maintainers (command table) +/* Guard against regressions by maintainers (command table) */ + void sanitize_command_list(void) { - struct xstate *x = xstatus(); + struct xstate *x = xstatus(0, NULL); unsigned long c; unsigned long num_commands; @@ -41,20 +41,18 @@ sanitize_command_list(void) sanitize_command_index(c); } -/* - * TODO: specific config checks per command +/* TODO: specific config checks per command */ + void sanitize_command_index(unsigned long c) { - struct xstate *x = xstatus(); - struct commands *cmd; + struct xstate *x = xstatus(0, NULL); + struct commands *cmd = &x->cmd[c]; int _flag; unsigned long gbe_rw_size; - cmd = &x->cmd[c]; - check_command_num(c); if (cmd->argc < 3) @@ -108,7 +106,7 @@ sanitize_command_index(unsigned long c) void set_cmd(int argc, char *argv[]) { - struct xstate *x = xstatus(); + struct xstate *x = xstatus(0, NULL); const char *cmd; unsigned long c; @@ -139,14 +137,10 @@ set_cmd(int argc, char *argv[]) void set_cmd_args(int argc, char *argv[]) { - struct xstate *x = xstatus(); - struct commands *cmd; - struct xfile *f; - unsigned long i; - - i = x->i; - cmd = &x->cmd[i]; - f = &x->f; + struct xstate *x = xstatus(0, NULL); + unsigned long i = x->i; + struct commands *cmd = &x->cmd[i]; + struct xfile *f = &x->f; if (!valid_command(i) || argc < 3) usage(); @@ -191,26 +185,6 @@ conv_argv_part_num(const char *part_str) return (unsigned long)(ch - '0'); } -void -run_cmd(void) -{ - struct xstate *x = xstatus(); - unsigned long i; - void (*run)(void); - - i = x->i; - run = x->cmd[i].run; - - check_command_num(i); - - if (run == NULL) - err(EINVAL, "Command %lu: null ptr", i); - - run(); - - for (i = 0; i < items(x->cmd); i++) - x->cmd[i].run = cmd_helper_err; -} void check_command_num(unsigned long c) @@ -223,7 +197,7 @@ check_command_num(unsigned long c) unsigned char valid_command(unsigned long c) { - struct xstate *x = xstatus(); + struct xstate *x = xstatus(0, NULL); struct commands *cmd; if (c >= items(x->cmd)) @@ -242,11 +216,10 @@ valid_command(unsigned long c) void cmd_helper_setmac(void) { - struct xstate *x = xstatus(); - unsigned long partnum; - struct macaddr *mac; + struct xstate *x = xstatus(0, NULL); + struct macaddr *mac = &x->mac; - mac = &x->mac; + unsigned long partnum; check_cmd(cmd_helper_setmac, "setmac"); @@ -260,13 +233,11 @@ cmd_helper_setmac(void) void parse_mac_string(void) { - struct xstate *x = xstatus(); - struct macaddr *mac; + struct xstate *x = xstatus(0, NULL); + struct macaddr *mac = &x->mac; unsigned long mac_byte; - mac = &x->mac; - if (xstrxlen(x->mac.str, 18) != 17) err(EINVAL, "MAC address is the wrong length"); @@ -285,16 +256,14 @@ parse_mac_string(void) void set_mac_byte(unsigned long mac_byte_pos) { - struct xstate *x = xstatus(); - struct macaddr *mac; + struct xstate *x = xstatus(0, NULL); + struct macaddr *mac = &x->mac; char separator; unsigned long mac_str_pos; unsigned long mac_nib_pos; - mac = &x->mac; - mac_str_pos = mac_byte_pos * 3; if (mac_str_pos < 15) { @@ -311,14 +280,12 @@ void set_mac_nib(unsigned long mac_str_pos, unsigned long mac_byte_pos, unsigned long mac_nib_pos) { - struct xstate *x = xstatus(); - struct macaddr *mac; + struct xstate *x = xstatus(0, NULL); + struct macaddr *mac = &x->mac; char mac_ch; unsigned short hex_num; - mac = &x->mac; - mac_ch = mac->str[mac_str_pos + mac_nib_pos]; if ((hex_num = hextonum(mac_ch)) > 15) @@ -345,15 +312,12 @@ set_mac_nib(unsigned long mac_str_pos, void write_mac_part(unsigned long partnum) { - struct xstate *x = xstatus(); - struct xfile *f; - struct macaddr *mac; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; + struct macaddr *mac = &x->mac; unsigned long w; - f = &x->f; - mac = &x->mac; - check_bin(partnum, "part number"); if (!f->part_valid[partnum]) return; @@ -369,13 +333,11 @@ write_mac_part(unsigned long partnum) void cmd_helper_dump(void) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; unsigned long p; - f = &x->f; - check_cmd(cmd_helper_dump, "dump"); f->part_valid[0] = good_checksum(0); @@ -408,10 +370,13 @@ print_mac_from_nvm(unsigned long partnum) unsigned short val16; for (c = 0; c < 3; c++) { + val16 = nvm_word(c, partnum); + printf("%02x:%02x", (unsigned int)(val16 & 0xff), (unsigned int)(val16 >> 8)); + if (c == 2) printf("\n"); else @@ -451,13 +416,11 @@ hexdump(unsigned long partnum) void cmd_helper_swap(void) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; check_cmd(cmd_helper_swap, "swap"); - f = &x->f; - x_v_memcpy( f->buf + (unsigned long)GBE_WORK_SIZE, f->buf, @@ -480,13 +443,11 @@ cmd_helper_swap(void) void cmd_helper_copy(void) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; check_cmd(cmd_helper_copy, "copy"); - f = &x->f; - x_v_memcpy( f->buf + (unsigned long)((f->part ^ 1) * GBE_PART_SIZE), f->buf + (unsigned long)(f->part * GBE_PART_SIZE), @@ -498,7 +459,8 @@ cmd_helper_copy(void) void cmd_helper_cat(void) { - struct xstate *x = xstatus(); + struct xstate *x = xstatus(0, NULL); + check_cmd(cmd_helper_cat, "cat"); x->cat = 0; @@ -508,7 +470,8 @@ cmd_helper_cat(void) void cmd_helper_cat16(void) { - struct xstate *x = xstatus(); + struct xstate *x = xstatus(0, NULL); + check_cmd(cmd_helper_cat16, "cat16"); x->cat = 1; @@ -518,7 +481,8 @@ cmd_helper_cat16(void) void cmd_helper_cat128(void) { - struct xstate *x = xstatus(); + struct xstate *x = xstatus(0, NULL); + check_cmd(cmd_helper_cat128, "cat128"); x->cat = 15; @@ -528,14 +492,12 @@ cmd_helper_cat128(void) void cat(unsigned long nff) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; unsigned long p; unsigned long ff; - f = &x->f; - p = 0; ff = 0; @@ -575,22 +537,15 @@ void check_cmd(void (*fn)(void), const char *name) { - struct xstate *x = xstatus(); - unsigned long i; + struct xstate *x = xstatus(0, NULL); + unsigned long i = x->i; - if (x->cmd[x->i].run != fn) + if (x->cmd[i].run != fn) err(ECANCELED, "Running %s, but cmd %s is set", - name, x->cmd[x->i].str); + name, x->cmd[i].str); - /* - * In addition to making sure we ran - * the right command, we now disable - * all commands from running again - * - * the _nop function will just call - * err() immediately + /* prevent second command */ - for (i = 0; i < items(x->cmd); i++) x->cmd[i].run = cmd_helper_err; } diff --git a/util/nvmutil/lib/file.c b/util/nvmutil/lib/file.c index 2638817d..92d7ee7c 100644 --- a/util/nvmutil/lib/file.c +++ b/util/nvmutil/lib/file.c @@ -24,21 +24,40 @@ #include "../include/common.h" -/* - * TODO: make generic. S_ISREG: check every other - * type, erring only if it doesn't match what was - * passed as type requested. - * also: - * have variable need_seek, only err on seek if - * need_seek is set. - * also consider the stat check in this generic - * context - * make tthe return type an int, not a void. - * return -1 with errno set to indicate error, - * though the syscalls mostly handle that. - * save errno before lseek, resetting it after - * the check if return >-1 +/* check that a file changed */ + +int +same_file(int fd, struct stat *st_old, + int check_size) +{ + struct stat st; + int saved_errno = errno; + + if (st_old == NULL || fd < 0) + goto err_same_file; + + if (fstat(fd, &st) == -1) + return -1; + + if (st.st_dev != st_old->st_dev || + st.st_ino != st_old->st_ino || + !S_ISREG(st.st_mode)) + goto err_same_file; + + if (check_size && + st.st_size != st_old->st_size) + goto err_same_file; + + errno = saved_errno; + return 0; + +err_same_file: + + errno = EIO; + return -1; +} + void xopen(int *fd_ptr, const char *path, int flags, struct stat *st) { @@ -55,10 +74,10 @@ xopen(int *fd_ptr, const char *path, int flags, struct stat *st) err(errno, "%s: file not seekable", path); } -/* - * Ensure rename() is durable by syncing the +/* Ensure x_i_rename() is durable by syncing the * directory containing the target file. */ + int fsync_dir(const char *path) { @@ -165,8 +184,7 @@ err_fsync_dir: return -1; } -/* - * create new tmpfile path +/* create new tmpfile path * * ON SUCCESS: * @@ -189,14 +207,14 @@ err_fsync_dir: * if local is zero, then 3rd arg (path) * is irrelevant and can be NULL */ + char * new_tmpfile(int *fd, int local, const char *path) { unsigned long maxlen; struct stat st; - /* - * please do not modify the + /* please do not modify the * strings or I will get mad */ char tmp_none[] = ""; @@ -250,8 +268,7 @@ new_tmpfile(int *fd, int local, const char *path) if (local) { base = tmp_none; - /* - * appended to filename for tmp: + /* appended to filename for tmp: */ tmpdir_len = xstrxlen(default_tmpname, maxlen); } else { @@ -270,8 +287,7 @@ new_tmpfile(int *fd, int local, const char *path) tmppath_len = tmpdir_len + tmpname_len; ++tmppath_len; /* for '/' or '.' */ - /* - * max length -1 of maxlen + /* max length -1 of maxlen * for termination */ if (tmpdir_len > maxlen - tmpname_len - 1) @@ -390,7 +406,6 @@ x_c_tmpdir(void) struct stat st; t = getenv("TMPDIR"); - t = getenv("TMPDIR"); if (t && *t) { if (stat(t, &st) == 0 && S_ISDIR(st.st_mode)) { @@ -409,9 +424,9 @@ x_c_tmpdir(void) return "."; } -/* - * portable mkstemp +/* portable mkstemp */ + int x_i_mkstemp(char *template) { @@ -436,7 +451,9 @@ x_i_mkstemp(char *template) for (i = 0; i < 100; i++) { for (j = 0; j < 6; j++) { + r = rlong(); + p[j] = ch[(unsigned long)(r >> 1) % (sizeof(ch) - 1)]; } @@ -466,8 +483,7 @@ x_i_mkstemp(char *template) * EINTR/EAGAIN looping is done indefinitely. */ -/* - * rw_file_exact() - Read perfectly or die +/* rw_file_exact() - Read perfectly or die * * Read/write, and absolutely insist on an * absolute read; e.g. if 100 bytes are @@ -483,6 +499,7 @@ x_i_mkstemp(char *template) * times upon zero-return, to recover, * otherwise it will return an error. */ + long rw_file_exact(int fd, unsigned char *mem, unsigned long nrw, off_t off, int rw_type, int loop_eagain, @@ -549,8 +566,7 @@ err_rw_file_exact: return -1; } -/* - * prw() - portable read-write +/* prw() - portable read-write * * This implements a portable analog of pwrite() * and pread() - note that this version is not @@ -809,19 +825,17 @@ err_is_file: return -1; } -/* - * Check overflows caused by buggy libc. +/* Check weirdness on buggy libc. * * POSIX can say whatever it wants. * specification != implementation */ + long rw_over_nrw(long r, unsigned long nrw) { - /* - * If a byte length of zero - * was requested, that is - * clearly a bug. No way. + /* not a libc bug, but we + * don't like the number zero */ if (!nrw) goto err_rw_over_nrw; @@ -832,8 +846,7 @@ rw_over_nrw(long r, unsigned long nrw) if ((unsigned long) r > X_LONG_MAX) { - /* - * Theoretical buggy libc + /* Theoretical buggy libc * check. Extremely academic. * * Specifications never @@ -843,12 +856,16 @@ rw_over_nrw(long r, unsigned long nrw) * * Check this after using * [p]read() or [p]write() + * + * NOTE: here, we assume + * long integers are the + * same size as SSIZE_T */ + goto err_rw_over_nrw; } - /* - * Theoretical buggy libc: + /* Theoretical buggy libc: * Should never return a number of * bytes above the requested length. */ @@ -888,35 +905,31 @@ lseek_loop(int fd, off_t off, int whence, } #endif -/* - * If a given error loop is enabled, - * e.g. EINTR or EAGAIN, an I/O operation - * will loop until errno isn't -1 and one - * of these, e.g. -1 and EINTR - */ int try_err(int loop_err, int errval) { if (loop_err) return errval; - /* errno is never negative, - so functions checking it - can use it accordingly */ return -1; } -/* - * non-atomic rename +/* portable rename(). WARNING: + * not powercut-safe. do this to + * use system rename: + * #define SYS_RENAME 1 * - * commented because i can't sacrifice - * exactly this property. nvmutil tries - * to protect files against e.g. power loss + * written academically, but in reality, + * nearly all unix systems have rename() */ -/* + int x_i_rename(const char *src, const char *dst) { +#if defined(SYS_RENAME) &&\ + SYS_RENAME > 0 + return rename(src, dst); +#else int sfd, dirfd; ssize_t r; char buf[8192]; @@ -955,8 +968,8 @@ x_i_rename(const char *src, const char *dst) return -1; return 0; +#endif } -*/ int x_i_close(int fd) diff --git a/util/nvmutil/lib/io.c b/util/nvmutil/lib/io.c index 99c9f04d..1a234b8f 100644 --- a/util/nvmutil/lib/io.c +++ b/util/nvmutil/lib/io.c @@ -3,8 +3,6 @@ * Copyright (c) 2026 Leah Rowe <leah@libreboot.org> * * I/O functions specific to nvmutil. - * - * Related: file.c */ #ifdef __OpenBSD__ @@ -29,30 +27,22 @@ void open_gbe_file(void) { - struct xstate *x = xstatus(); - struct commands *cmd; - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct commands *cmd = &x->cmd[x->i]; + struct xfile *f = &x->f; - struct stat _st; int _flags; - cmd = &x->cmd[x->i]; - f = &x->f; - xopen(&f->gbe_fd, f->fname, cmd->flags | O_BINARY | - O_NOFOLLOW | O_CLOEXEC, &_st); + O_NOFOLLOW | O_CLOEXEC, &f->gbe_st); - /* inode will be checked later on write */ - f->gbe_dev = _st.st_dev; - f->gbe_ino = _st.st_ino; - - if (_st.st_nlink > 1) + if (f->gbe_st.st_nlink > 1) err(EINVAL, "%s: warning: file has multiple (%lu) hard links\n", - f->fname, (unsigned long)_st.st_nlink); + f->fname, (unsigned long)f->gbe_st.st_nlink); - if (_st.st_nlink == 0) + if (f->gbe_st.st_nlink == 0) err(EIO, "%s: file unlinked while open", f->fname); _flags = fcntl(f->gbe_fd, F_GETFL); @@ -68,7 +58,7 @@ open_gbe_file(void) if (_flags & O_APPEND) err(EIO, "%s: O_APPEND flag", f->fname); - f->gbe_file_size = _st.st_size; + f->gbe_file_size = f->gbe_st.st_size; switch (f->gbe_file_size) { case SIZE_8KB: @@ -83,37 +73,14 @@ open_gbe_file(void) err(errno, "%s: can't lock", f->fname); } -/* - * We copy the entire gbe file - * to the tmpfile, and then we - * work on that. We copy back - * afterward. this is the copy. - * - * we copy to tmpfile even on - * read-only commands, for the - * double-read verification, - * which also benefits cmd_cat. - */ void copy_gbe(void) { - struct xstate *x = xstatus(); - struct xfile *f; - - f = &x->f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; read_file(); - /* - regular operations post-read operate only on the first - 8KB, because each GbE part is the first 4KB of each - half of the file. - - we no longer care about anything past 8KB, until we get - to writing, at which point we will flush the buffer - again - */ - if (f->gbe_file_size == SIZE_8KB) return; @@ -125,15 +92,14 @@ copy_gbe(void) void read_file(void) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; struct stat _st; long _r; - f = &x->f; - - /* read main file */ + /* read main file + */ _r = rw_file_exact(f->gbe_fd, f->buf, f->gbe_file_size, 0, IO_PREAD, NO_LOOP_EAGAIN, LOOP_EINTR, MAX_ZERO_RW_RETRY, OFF_ERR); @@ -141,8 +107,8 @@ read_file(void) if (_r < 0) err(errno, "%s: read failed", f->fname); - /* copy to tmpfile */ - + /* copy to tmpfile + */ _r = rw_file_exact(f->tmp_fd, f->buf, f->gbe_file_size, 0, IO_PWRITE, NO_LOOP_EAGAIN, LOOP_EINTR, MAX_ZERO_RW_RETRY, OFF_ERR); @@ -151,10 +117,8 @@ read_file(void) err(errno, "%s: %s: copy failed", f->fname, f->tname); - /* - * file size comparison + /* file size comparison */ - if (fstat(f->tmp_fd, &_st) == -1) err(errno, "%s: stat", f->tname); @@ -164,9 +128,7 @@ read_file(void) err(EIO, "%s: %s: not the same size", f->fname, f->tname); - /* - * fsync tmp gbe file, because we will compare - * its contents to what was read (for safety) + /* needs sync, for verification */ if (x_i_fsync(f->tmp_fd) == -1) err(errno, "%s: fsync (tmpfile copy)", f->tname); @@ -182,42 +144,25 @@ read_file(void) err(errno, "%s: %s: read contents differ (pre-test)", f->fname, f->tname); } + void write_gbe_file(void) { - struct xstate *x = xstatus(); - struct commands *cmd; - struct xfile *f; - - struct stat _gbe_st; - struct stat _tmp_st; + struct xstate *x = xstatus(0, NULL); + struct commands *cmd = &x->cmd[x->i]; + struct xfile *f = &x->f; unsigned long p; unsigned char update_checksum; - cmd = &x->cmd[x->i]; - f = &x->f; - if ((cmd->flags & O_ACCMODE) == O_RDONLY) return; - if (fstat(f->gbe_fd, &_gbe_st) == -1) - err(errno, "%s: re-check", f->fname); - if (_gbe_st.st_dev != f->gbe_dev || _gbe_st.st_ino != f->gbe_ino) - err(EIO, "%s: file replaced while open", f->fname); - if (_gbe_st.st_size != f->gbe_file_size) - err(errno, "%s: file size changed before write", f->fname); - if (!S_ISREG(_gbe_st.st_mode)) - err(errno, "%s: file type changed before write", f->fname); - - if (fstat(f->tmp_fd, &_tmp_st) == -1) - err(errno, "%s: re-check", f->tname); - if (_tmp_st.st_dev != f->tmp_dev || _tmp_st.st_ino != f->tmp_ino) - err(EIO, "%s: file replaced while open", f->tname); - if (_tmp_st.st_size != f->gbe_file_size) - err(errno, "%s: file size changed before write", f->tname); - if (!S_ISREG(_tmp_st.st_mode)) - err(errno, "%s: file type changed before write", f->tname); + if (same_file(f->tmp_fd, &f->tmp_st, 0) < 0) + err(errno, "%s: file inode/device changed", f->tname); + + if (same_file(f->gbe_fd, &f->gbe_st, 1) < 0) + err(errno, "%s: file has changed", f->fname); update_checksum = cmd->chksum_write; @@ -236,9 +181,9 @@ void rw_gbe_file_part(unsigned long p, int rw_type, const char *rw_type_str) { - struct xstate *x = xstatus(); - struct commands *cmd; - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct commands *cmd = &x->cmd[x->i]; + struct xfile *f = &x->f; long rval; @@ -247,9 +192,6 @@ rw_gbe_file_part(unsigned long p, int rw_type, unsigned long gbe_rw_size; unsigned char *mem_offset; - cmd = &x->cmd[x->i]; - f = &x->f; - gbe_rw_size = cmd->rw_size; if (rw_type < IO_PREAD || rw_type > IO_PWRITE) @@ -274,16 +216,13 @@ rw_gbe_file_part(unsigned long p, int rw_type, void write_to_gbe_bin(void) { - struct xstate *x = xstatus(); - struct commands *cmd; - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct commands *cmd = &x->cmd[x->i]; + struct xfile *f = &x->f; int saved_errno; int mv; - cmd = &x->cmd[x->i]; - f = &x->f; - if ((cmd->flags & O_ACCMODE) != O_RDWR) return; @@ -338,11 +277,9 @@ write_to_gbe_bin(void) fprintf(stderr, "%s: %s\n", f->fname, strerror(errno)); } else { - /* - * tmpfile removed - * by the rename - */ + /* removed by rename + */ if (f->tname != NULL) free(f->tname); @@ -350,12 +287,6 @@ write_to_gbe_bin(void) } } - /* - * finally: - * must sync to disk! - * very nearly done - */ - if (!f->io_err_gbe_bin) return; @@ -369,9 +300,9 @@ write_to_gbe_bin(void) void check_written_part(unsigned long p) { - struct xstate *x = xstatus(); - struct commands *cmd; - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct commands *cmd = &x->cmd[x->i]; + struct xfile *f = &x->f; long rval; @@ -380,12 +311,8 @@ check_written_part(unsigned long p) off_t file_offset; unsigned char *mem_offset; - struct stat st; unsigned char *buf_restore; - cmd = &x->cmd[x->i]; - f = &x->f; - if (!f->part_modified[p]) return; @@ -396,15 +323,11 @@ check_written_part(unsigned long p) memset(f->pad, 0xff, sizeof(f->pad)); - if (fstat(f->gbe_fd, &st) == -1) - err(errno, "%s: fstat (post-write)", f->fname); - if (st.st_dev != f->gbe_dev || st.st_ino != f->gbe_ino) - err(EIO, "%s: file changed during write", f->fname); + if (same_file(f->tmp_fd, &f->tmp_st, 0) < 0) + err(errno, "%s: file inode/device changed", f->tname); - if (fstat(f->tmp_fd, &st) == -1) - err(errno, "%s: fstat (post-write)", f->tname); - if (st.st_dev != f->tmp_dev || st.st_ino != f->tmp_ino) - err(EIO, "%s: file changed during write", f->tname); + if (same_file(f->gbe_fd, &f->gbe_st, 1) < 0) + err(errno, "%s: file changed during write", f->fname); rval = rw_gbe_file_exact(f->tmp_fd, f->pad, gbe_rw_size, file_offset, IO_PREAD); @@ -442,13 +365,11 @@ check_written_part(unsigned long p) void report_io_err_rw(void) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; unsigned long p; - f = &x->f; - if (!f->io_err_gbe) return; @@ -500,8 +421,8 @@ report_io_err_rw(void) int gbe_mv(void) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; int rval; @@ -511,8 +432,6 @@ gbe_mv(void) char *dest_tmp; int dest_fd; - f = &x->f; - /* will be set 0 if it doesn't */ tmp_gbe_bin_exists = 1; @@ -521,7 +440,7 @@ gbe_mv(void) saved_errno = errno; - rval = rename(f->tname, f->fname); + rval = x_i_rename(f->tname, f->fname); if (rval > -1) { /* @@ -576,7 +495,7 @@ gbe_mv(void) if (x_i_close(dest_fd) == -1) goto ret_gbe_mv; - if (rename(dest_tmp, f->fname) == -1) + if (x_i_rename(dest_tmp, f->fname) == -1) goto ret_gbe_mv; if (fsync_dir(f->fname) < 0) { @@ -640,13 +559,11 @@ ret_gbe_mv: unsigned char * gbe_mem_offset(unsigned long p, const char *f_op) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; off_t gbe_off; - f = &x->f; - gbe_off = gbe_x_offset(p, f_op, "mem", GBE_PART_SIZE, GBE_WORK_SIZE); @@ -664,13 +581,11 @@ gbe_mem_offset(unsigned long p, const char *f_op) off_t gbe_file_offset(unsigned long p, const char *f_op) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; off_t gbe_file_half_size; - f = &x->f; - gbe_file_half_size = f->gbe_file_size >> 1; return gbe_x_offset(p, f_op, "file", @@ -681,15 +596,13 @@ off_t gbe_x_offset(unsigned long p, const char *f_op, const char *d_type, off_t nsize, off_t ncmp) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; off_t off; check_bin(p, "part number"); - f = &x->f; - off = ((off_t)p) * (off_t)nsize; if (off > ncmp - GBE_PART_SIZE) @@ -707,13 +620,11 @@ long rw_gbe_file_exact(int fd, unsigned char *mem, unsigned long nrw, off_t off, int rw_type) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; long r; - f = &x->f; - if (io_args(fd, mem, nrw, off, rw_type) == -1) return -1; diff --git a/util/nvmutil/lib/num.c b/util/nvmutil/lib/num.c index dd6e9901..374cc9a0 100644 --- a/util/nvmutil/lib/num.c +++ b/util/nvmutil/lib/num.c @@ -45,9 +45,7 @@ hextonum(char ch_s) return 16; /* invalid character */ } -/* - * Portable random - * number generator +/* Random numbers */ unsigned long rlong(void) diff --git a/util/nvmutil/lib/state.c b/util/nvmutil/lib/state.c index ec420594..f4f83e48 100644 --- a/util/nvmutil/lib/state.c +++ b/util/nvmutil/lib/state.c @@ -5,13 +5,6 @@ * This tool lets you modify Intel GbE NVM (Gigabit Ethernet * Non-Volatile Memory) images, e.g. change the MAC address. * These images configure your Intel Gigabit Ethernet adapter. - * - * This code is designed to be portable, running on as many - * Unix and Unix-like systems as possible (mainly BSD/Linux). - * - * Recommended CFLAGS for Clang/GCC: - * - * -Os -Wall -Wextra -Werror -pedantic -std=c90 */ #ifdef __OpenBSD__ @@ -34,40 +27,16 @@ #include "../include/common.h" /* - * Program state/command table - * Default config stored here, - * and copied to a newly allocated - * buffer in memory, then the pointer - * is passed. The rest of the program - * will manipulate this data. - */ -/* -TODO: -eventually, i will not have this return -a pointer at all. instead, a similar key -mechanism will be used for other access -functions e.g. word/set_word, err(needs -to clean up), and so on. then those -would return values if already initialised, -but would not permit additional init - will -decide exactly how to implement this at a -later state. - -this is part of an ongoing effort to introduce -extreme memory safety into this program. + * Initialise program state, + * load GbE file and verify + * data, ready for operation + * (singleton design) */ struct xstate * -xstatus(void) +xstatus(int argc, char *argv[]) { - static int first_run = 1; - static struct xstate us = { - /* .cmd (update cmd[] in the struct if adding to it) - DO NOT FORGET. or C will init zeroes/NULLs */ - - /* cmd[] members */ - /* DO NOT MESS THIS UP */ - /* items must be set *exactly* */ + /* DO NOT MESS THIS UP, OR THERE WILL BE DEMONS */ { { CMD_DUMP, "dump", cmd_helper_dump, ARGC_3, @@ -122,36 +91,165 @@ xstatus(void) /* .no_cmd (set 0 when a command is found) */ 1, - /* .xsize (size of the stuct will be stored here later) */ - 0, - /* .cat (cat helpers set this) */ -1 }; + static int first_run = 1; + if (!first_run) return &us; + us.f.buf = us.f.real_buf; + first_run = 0; + us.argv0 = argv[0]; + + if (argc > 1) + us.f.fname = argv[1]; + + if (argc < 3) + usage(); + +/* https://man.openbsd.org/pledge.2 + https://man.openbsd.org/unveil.2 */ +#if defined(__OpenBSD__) && defined(OpenBSD) +#if (OpenBSD) >= 604 + if (pledge("stdio flock rpath wpath cpath unveil", NULL) == -1) + err(errno, "pledge plus unveil"); +#elif (OpenBSD) >= 509 + if (pledge("stdio flock rpath wpath cpath", NULL) == -1) + err(errno, "pledge"); +#endif +#endif - us.xsize = sizeof(us); +#ifndef S_ISREG + err(ECANCELED, "Can't determine file types (S_ISREG undefined)"); +#endif - us.f.buf = us.f.real_buf; +#ifndef CHAR_BIT + err(ECANCELED, "Unknown char size"); +#else + if (CHAR_BIT != 8) + err(EINVAL, "Unsupported char size"); +#endif + +#if defined(__OpenBSD__) && defined(OpenBSD) && \ + (OpenBSD) >= 604 + /* can only use local tmp on openbsd, due to unveil */ + us.f.tname = new_tmpfile(&us.f.tmp_fd, 1, NULL); +#else + us.f.tname = new_tmpfile(&us.f.tmp_fd, 0, NULL); +#endif + if (us.f.tname == NULL) + err(errno, "Can't create tmpfile"); + if (*us.f.tname == '\0') + err(errno, "tmp dir is an empty string"); + +#if defined(__OpenBSD__) && defined(OpenBSD) && \ + OpenBSD >= 604 + if (unveil(f->tname, "rwc") == -1) + err(errno, "unveil rwc: %s", f->tname); +#endif + if (fstat(us.f.tmp_fd, &us.f.tmp_st) < 0) + err(errno, "%s: stat", us.f.tname); + + sanitize_command_list(); + + /* parse user command */ + set_cmd(argc, argv); + set_cmd_args(argc, argv); + +#if defined(__OpenBSD__) && defined(OpenBSD) && \ + (OpenBSD) >= 604 + if ((us.cmd[i].flags & O_ACCMODE) == O_RDONLY) { + if (unveil(us.f.fname, "r") == -1) + err(errno, "%s: unveil r", us.f.fname); + } else { + if (unveil(us.f.fname, "rwc") == -1) + err(errno, "%s: unveil rw", us.f.tname); + } + + if (unveil(us.f.tname, "rwc") == -1) + err(errno, "%s: unveil rwc", us.f.tname); + + if (unveil(NULL, NULL) == -1) + err(errno, "unveil block (rw)"); - us.f.gbe_fd = -1; - us.f.tmp_fd = -1; + if (pledge("stdio flock rpath wpath cpath", NULL) == -1) + err(errno, "pledge (kill unveil)"); +#endif + + open_gbe_file(); + + memset(us.f.real_buf, 0, sizeof(us.f.real_buf)); + memset(us.f.bufcmp, 0, sizeof(us.f.bufcmp)); + + /* for good measure */ + memset(us.f.pad, 0, sizeof(us.f.pad)); - us.f.tname = NULL; - us.f.fname = NULL; + copy_gbe(); + read_checksums(); return &us; } +void +err(int nvm_errval, const char *msg, ...) +{ + struct xstate *x = xstatus(0, NULL); + + va_list args; + + if (errno == 0) + errno = nvm_errval; + if (!errno) + errno = ECANCELED; + + (void)exit_cleanup(); + + if (x != NULL) + fprintf(stderr, "%s: ", getnvmprogname()); + + va_start(args, msg); + vfprintf(stderr, msg, args); + va_end(args); + + fprintf(stderr, ": %s\n", strerror(errno)); + + exit(EXIT_FAILURE); +} + +const char * +getnvmprogname(void) +{ + struct xstate *x = xstatus(0, NULL); + + const char *p; + static char fallback[] = "nvmutil"; + + char *rval = fallback; + + if (x != NULL) { + if (x->argv0 == NULL || *x->argv0 == '\0') + return ""; + + rval = x->argv0; + } + + p = x_c_strrchr(rval, '/'); + + if (p) + return p + 1; + else + return rval; +} + int exit_cleanup(void) { - struct xstate *x = xstatus(); + struct xstate *x = xstatus(0, NULL); struct xfile *f; int close_err; diff --git a/util/nvmutil/lib/string.c b/util/nvmutil/lib/string.c index 529dbf59..517f490b 100644 --- a/util/nvmutil/lib/string.c +++ b/util/nvmutil/lib/string.c @@ -93,67 +93,14 @@ xstrxlen(const char *scmp, unsigned long maxlen) return xstr_index; } -void -err(int nvm_errval, const char *msg, ...) -{ - struct xstate *x = xstatus(); - - va_list args; - - if (errno == 0) - errno = nvm_errval; - if (!errno) - errno = ECANCELED; - - (void)exit_cleanup(); - - if (x != NULL) - fprintf(stderr, "%s: ", getnvmprogname()); - - va_start(args, msg); - vfprintf(stderr, msg, args); - va_end(args); - - fprintf(stderr, ": %s\n", strerror(errno)); - - exit(EXIT_FAILURE); -} - -const char * -getnvmprogname(void) -{ - struct xstate *x = xstatus(); - - const char *p; - static char fallback[] = "nvmutil"; - - char *rval = fallback; - - if (x != NULL) { - if (x->argv0 == NULL || *x->argv0 == '\0') - return ""; - - rval = x->argv0; - } - - p = x_c_strrchr(rval, '/'); - - if (p) - return p + 1; - else - return rval; -} - char * x_c_strrchr(const char *s, int c) { const char *p = NULL; - while (*s) { + for ( ; *s; s++) if (*s == (char)c) p = s; - s++; - } if (c == '\0') return (char *)s; diff --git a/util/nvmutil/lib/word.c b/util/nvmutil/lib/word.c index 4a8280ba..4647c1f4 100644 --- a/util/nvmutil/lib/word.c +++ b/util/nvmutil/lib/word.c @@ -29,20 +29,17 @@ * GbE NVM files store 16-bit (2-byte) little-endian words. * We must therefore swap the order when reading or writing. * - * NOTE: The MAC address words are stored big-endian in the - * file, but we assume otherwise and adapt accordingly. + * NOTE: The MAC address words are stored big-endian in-file. */ unsigned short nvm_word(unsigned long pos16, unsigned long p) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; unsigned long pos; - f = &x->f; - check_nvm_bound(pos16, p); pos = (pos16 << 1) + (p * GBE_PART_SIZE); @@ -53,13 +50,11 @@ nvm_word(unsigned long pos16, unsigned long p) void set_nvm_word(unsigned long pos16, unsigned long p, unsigned short val16) { - struct xstate *x = xstatus(); - struct xfile *f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; unsigned long pos; - f = &x->f; - check_nvm_bound(pos16, p); pos = (pos16 << 1) + (p * GBE_PART_SIZE); @@ -72,10 +67,8 @@ set_nvm_word(unsigned long pos16, unsigned long p, unsigned short val16) void set_part_modified(unsigned long p) { - struct xstate *x = xstatus(); - struct xfile *f; - - f = &x->f; + struct xstate *x = xstatus(0, NULL); + struct xfile *f = &x->f; check_bin(p, "part number"); f->part_modified[p] = 1; @@ -84,10 +77,7 @@ set_part_modified(unsigned long p) void check_nvm_bound(unsigned long c, unsigned long p) { - /* - * NVM_SIZE assumed as the limit, because this - * current design assumes that we will only - * ever modified the NVM area. + /* Block out of bound NVM access */ check_bin(p, "part number"); diff --git a/util/nvmutil/nvmutil.c b/util/nvmutil/nvmutil.c index 5217dd5f..8b4f01d1 100644 --- a/util/nvmutil/nvmutil.c +++ b/util/nvmutil/nvmutil.c @@ -5,9 +5,6 @@ * This tool lets you modify Intel GbE NVM (Gigabit Ethernet * Non-Volatile Memory) images, e.g. change the MAC address. * These images configure your Intel Gigabit Ethernet adapter. - * - * This code is designed to be portable, running on as many - * Unix and Unix-like systems as possible (mainly BSD/Linux). */ #ifdef __OpenBSD__ @@ -32,115 +29,19 @@ int main(int argc, char *argv[]) { - struct xstate *x; - struct commands *cmd; - struct xfile *f; - unsigned long i; - - char *tmp_path; - - struct stat st; - int fd; - - tmp_path = NULL; - -#ifndef S_ISREG - err(ECANCELED, "Can't determine file types (S_ISREG undefined)"); -#endif - -#ifndef CHAR_BIT - err(ECANCELED, "Unknown char size"); -#else - if (CHAR_BIT != 8) - err(EINVAL, "Unsupported char size"); -#endif - - if (argc < 3) - usage(); - -#ifdef NVMUTIL_UNVEIL - /* - * if global tmp is a different filesystem, - * unveil would trap on final file rename - * and we can't know the path in advance - */ - tmp_path = new_tmpfile(&fd, 1, NULL); -#else - tmp_path = new_tmpfile(&fd, 0, NULL); -#endif - - if (tmp_path == NULL) - err(errno, "Can't create tmpfile"); - -#ifdef NVMUTIL_PLEDGE -#ifdef NVMUTIL_UNVEIL - if (pledge("stdio flock rpath wpath cpath unveil", NULL) == -1) - err(errno, "pledge, unveil"); - if (unveil("/dev/null", "r") == -1) - err(errno, "unveil: /dev/null"); -#else - if (pledge("stdio flock rpath wpath cpath", NULL) == -1) - err(errno, "pledge"); -#endif -#endif - - x = xstatus(); - - if (x == NULL) - err(errno, NULL); - if (x->f.buf == NULL) - err(EINVAL, "Work buffer not initialised"); - - x->argv0 = argv[0]; - f = &x->f; - - f->fname = argv[1]; - f->tname = tmp_path; - f->tmp_fd = fd; - - if(fstat(fd, &st) < 0) - err(errno, "can't stat tmpfile"); - - f->tmp_dev = st.st_dev; - f->tmp_ino = st.st_ino; - - sanitize_command_list(); - - set_cmd(argc, argv); - set_cmd_args(argc, argv); - - i = x->i; - cmd = &x->cmd[i]; - -#ifdef NVMUTIL_UNVEIL - if ((cmd->flags & O_ACCMODE) == O_RDONLY) { - if (unveil(f->fname, "r") == -1) - err(errno, "%s: unveil r", f->fname); - } else { - if (unveil(f->tname, "rwc") == -1) - err(errno, "%s: unveil rw", f->tname); - } - - if (unveil(f->tname, "rwc") == -1) - err(errno, "%s: unveil rwc", f->tname); - - if (unveil(NULL, NULL) == -1) - err(errno, "unveil block (rw)"); - - if (pledge("stdio flock rpath wpath cpath", NULL) == -1) - err(errno, "pledge (kill unveil)"); -#endif - - open_gbe_file(); + struct xstate *x = xstatus(argc, argv); + struct commands *cmd = &x->cmd[x->i]; + struct xfile *f = &x->f; - memset(f->buf, 0, GBE_BUF_SIZE); - memset(f->bufcmp, 0, GBE_BUF_SIZE); + unsigned long c; - copy_gbe(); + if (cmd->run == NULL) + err(errno, "Command not set"); - read_checksums(); + cmd->run(); - run_cmd(); + for (c = 0; c < items(x->cmd); c++) + x->cmd[c].run = cmd_helper_err; if ((cmd->flags & O_ACCMODE) == O_RDWR) write_to_gbe_bin(); diff --git a/util/nvmutil/todo.c b/util/nvmutil/todo.c deleted file mode 100644 index 3b80dd83..00000000 --- a/util/nvmutil/todo.c +++ /dev/null @@ -1,134 +0,0 @@ -/* SPDX-License-Identifier: MIT - * - * Copyright (c) 2026 Leah Rowe <leah@libreboot.org> - * - * Five Year Plan - */ - -/* - * 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: ux 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 / 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, x_i_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. 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 seed e.g. 1234 -*/ -/* -TODO: stricter build flags, e.g. -CFLAGS += -fstack-protector-strong -CFLAGS += -fno-common -CFLAGS += -D_FORTIFY_SOURCE=2 -CFLAGS += -fPIE - -also consider: --fstack-clash-protection --Wl,-z,relro --Wl,-z,now -*/ - |
