diff options
Diffstat (limited to 'util/libreboot-utils/lib')
| -rw-r--r-- | util/libreboot-utils/lib/checksum.c | 4 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/command.c | 58 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/file.c | 173 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/io.c | 102 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/mkhtemp.c | 97 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/num.c | 2 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/rand.c | 20 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/state.c | 92 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/string.c | 83 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/usage.c | 2 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/word.c | 2 |
11 files changed, 289 insertions, 346 deletions
diff --git a/util/libreboot-utils/lib/checksum.c b/util/libreboot-utils/lib/checksum.c index 9a041989..97b0efca 100644 --- a/util/libreboot-utils/lib/checksum.c +++ b/util/libreboot-utils/lib/checksum.c @@ -59,10 +59,10 @@ read_checksums(void) if (_num_invalid >= _max_invalid) { if (_max_invalid == 1) - b0rk(ECANCELED, "%s: part %lu has a bad checksum", + err_exit(ECANCELED, "%s: part %lu has a bad checksum", f->fname, (size_t)f->part); - b0rk(ECANCELED, "%s: No valid checksum found in file", + err_exit(ECANCELED, "%s: No valid checksum found in file", f->fname); } } diff --git a/util/libreboot-utils/lib/command.c b/util/libreboot-utils/lib/command.c index c7048a23..d0f783dd 100644 --- a/util/libreboot-utils/lib/command.c +++ b/util/libreboot-utils/lib/command.c @@ -46,27 +46,27 @@ sanitize_command_index(size_t c) check_command_num(c); if (cmd->argc < 3) - b0rk(EINVAL, "cmd index %lu: argc below 3, %d", + err_exit(EINVAL, "cmd index %lu: argc below 3, %d", (size_t)c, cmd->argc); if (cmd->str == NULL) - b0rk(EINVAL, "cmd index %lu: NULL str", + err_exit(EINVAL, "cmd index %lu: NULL str", (size_t)c); if (*cmd->str == '\0') - b0rk(EINVAL, "cmd index %lu: empty str", + err_exit(EINVAL, "cmd index %lu: empty str", (size_t)c); if (slen(cmd->str, MAX_CMD_LEN +1, &rval) < 0) - b0rk(errno, "Could not get command length"); + err_exit(errno, "Could not get command length"); if (rval > MAX_CMD_LEN) { - b0rk(EINVAL, "cmd index %lu: str too long: %s", + err_exit(EINVAL, "cmd index %lu: str too long: %s", (size_t)c, cmd->str); } if (cmd->run == NULL) - b0rk(EINVAL, "cmd index %lu: cmd ptr null", + err_exit(EINVAL, "cmd index %lu: cmd ptr null", (size_t)c); check_bin(cmd->arg_part, "cmd.arg_part"); @@ -80,19 +80,19 @@ sanitize_command_index(size_t c) case NVM_SIZE: break; default: - b0rk(EINVAL, "Unsupported rw_size: %lu", + err_exit(EINVAL, "Unsupported rw_size: %lu", (size_t)gbe_rw_size); } if (gbe_rw_size > GBE_PART_SIZE) - b0rk(EINVAL, "rw_size larger than GbE part: %lu", + err_exit(EINVAL, "rw_size larger than GbE part: %lu", (size_t)gbe_rw_size); _flag = (cmd->flags & O_ACCMODE); if (_flag != O_RDONLY && _flag != O_RDWR) - b0rk(EINVAL, "invalid cmd.flags setting"); + err_exit(EINVAL, "invalid cmd.flags setting"); } void @@ -110,7 +110,7 @@ set_cmd(int argc, char *argv[]) cmd = x->cmd[c].str; if (scmp(argv[2], cmd, MAX_CMD_LEN, &rval) < 0) - err_no_cleanup(0, EINVAL, + err_exit(EINVAL, "could not compare command strings"); if (rval != 0) continue; /* not the right command */ @@ -123,7 +123,7 @@ set_cmd(int argc, char *argv[]) return; } - err_no_cleanup(0, EINVAL, + err_exit(EINVAL, "Too few args on command '%s'", cmd); } @@ -148,11 +148,11 @@ set_cmd_args(int argc, char *argv[]) /* Maintainer bug */ if (cmd->arg_part && argc < 4) - b0rk(EINVAL, + err_exit(EINVAL, "arg_part set for command that needs argc4"); if (cmd->arg_part && i == CMD_SETMAC) - b0rk(EINVAL, + err_exit(EINVAL, "arg_part set on CMD_SETMAC"); if (i == CMD_SETMAC) { @@ -174,13 +174,13 @@ conv_argv_part_num(const char *part_str) unsigned char ch; if (part_str[0] == '\0' || part_str[1] != '\0') - b0rk(EINVAL, "Partnum string '%s' wrong length", part_str); + err_exit(EINVAL, "Partnum string '%s' wrong length", part_str); /* char signedness is implementation-defined */ ch = (unsigned char)part_str[0]; if (ch < '0' || ch > '1') - b0rk(EINVAL, "Bad part number (%c)", ch); + err_exit(EINVAL, "Bad part number (%c)", ch); return (size_t)(ch - '0'); } @@ -189,7 +189,7 @@ void check_command_num(size_t c) { if (!valid_command(c)) - b0rk(EINVAL, "Invalid run_cmd arg: %lu", + err_exit(EINVAL, "Invalid run_cmd arg: %lu", (size_t)c); } @@ -205,7 +205,7 @@ valid_command(size_t c) cmd = &x->cmd[c]; if (c != cmd->chk) - b0rk(EINVAL, + err_exit(EINVAL, "Invalid cmd chk value (%lu) vs arg: %lu", cmd->chk, c); @@ -240,10 +240,10 @@ parse_mac_string(void) size_t rval; if (slen(x->mac.str, 18, &rval) < 0) - b0rk(EINVAL, "Could not determine MAC length"); + err_exit(EINVAL, "Could not determine MAC length"); if (rval != 17) - b0rk(EINVAL, "MAC address is the wrong length"); + err_exit(EINVAL, "MAC address is the wrong length"); memset(mac->mac_buf, 0, sizeof(mac->mac_buf)); @@ -251,10 +251,10 @@ parse_mac_string(void) set_mac_byte(mac_byte); if ((mac->mac_buf[0] | mac->mac_buf[1] | mac->mac_buf[2]) == 0) - b0rk(EINVAL, "Must not specify all-zeroes MAC address"); + err_exit(EINVAL, "Must not specify all-zeroes MAC address"); if (mac->mac_buf[0] & 1) - b0rk(EINVAL, "Must not specify multicast MAC address"); + err_exit(EINVAL, "Must not specify multicast MAC address"); } void @@ -272,7 +272,7 @@ set_mac_byte(size_t mac_byte_pos) if (mac_str_pos < 15) { if ((separator = mac->str[mac_str_pos + 2]) != ':') - b0rk(EINVAL, "Invalid MAC address separator '%c'", + err_exit(EINVAL, "Invalid MAC address separator '%c'", separator); } @@ -294,9 +294,9 @@ set_mac_nib(size_t mac_str_pos, if ((hex_num = hextonum(mac_ch)) > 15) { if (hex_num >= 17) - b0rk(EIO, "Randomisation failure"); + err_exit(EIO, "Randomisation failure"); else - b0rk(EINVAL, "Invalid character '%c'", + err_exit(EINVAL, "Invalid character '%c'", mac->str[mac_str_pos + mac_nib_pos]); } @@ -509,7 +509,7 @@ cat(size_t nff) if ((size_t)x->cat != nff) { - b0rk(ECANCELED, "erroneous call to cat"); + err_exit(ECANCELED, "erroneous call to cat"); } fflush(NULL); @@ -532,12 +532,12 @@ void cat_buf(unsigned char *b) { if (b == NULL) - b0rk(errno, "null pointer in cat command"); + err_exit(errno, "null pointer in cat command"); if (rw_file_exact(STDOUT_FILENO, b, GBE_PART_SIZE, 0, IO_WRITE, LOOP_EAGAIN, LOOP_EINTR, MAX_ZERO_RW_RETRY, OFF_ERR) < 0) - b0rk(errno, "stdout: cat"); + err_exit(errno, "stdout: cat"); } void check_cmd(void (*fn)(void), @@ -547,7 +547,7 @@ check_cmd(void (*fn)(void), size_t i = x->i; if (x->cmd[i].run != fn) - b0rk(ECANCELED, "Running %s, but cmd %s is set", + err_exit(ECANCELED, "Running %s, but cmd %s is set", name, x->cmd[i].str); /* prevent second command @@ -559,6 +559,6 @@ check_cmd(void (*fn)(void), void cmd_helper_err(void) { - b0rk(ECANCELED, + err_exit(ECANCELED, "Erroneously running command twice"); } diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c index 3b3e57d8..3620f425 100644 --- a/util/libreboot-utils/lib/file.c +++ b/util/libreboot-utils/lib/file.c @@ -62,27 +62,7 @@ same_file(int fd, struct stat *st_old, return 0; err_same_file: - - if (errno == saved_errno) - errno = ESTALE; - - return -1; -} - -void -xopen(int *fd_ptr, const char *path, int flags, struct stat *st) -{ - if ((*fd_ptr = open(path, flags)) < 0) - err_no_cleanup(0, errno, "%s", path); - - if (fstat(*fd_ptr, st) < 0) - err_no_cleanup(0, errno, "%s: stat", path); - - if (!S_ISREG(st->st_mode)) - err_no_cleanup(0, errno, "%s: not a regular file", path); - - if (lseek_on_eintr(*fd_ptr, 0, SEEK_CUR, 1, 1) == (off_t)-1) - err_no_cleanup(0, errno, "%s: file not seekable", path); + return set_errno(saved_errno, ESTALE); } int @@ -146,11 +126,7 @@ fsync_dir(const char *path) if_err_sys(fsync_on_eintr(dirfd) == -1)) goto err_fsync_dir; - if (close_on_eintr(dirfd) == -1) { - - dirfd = -1; - goto err_fsync_dir; - } + close_on_eintr(&dirfd); free_and_set_null(&dirbuf); @@ -159,13 +135,11 @@ fsync_dir(const char *path) err_fsync_dir: - if (errno == saved_errno) - errno = EIO; free_and_set_null(&dirbuf); - close_no_err(&dirfd); + close_on_eintr(&dirfd); - return -1; + return set_errno(saved_errno, EIO); } /* rw_file_exact() - Read perfectly or die @@ -268,10 +242,7 @@ rw_file_exact(int fd, unsigned char *mem, size_t nrw, err_rw_file_exact: - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } /* prw() - portable read-write with more @@ -450,11 +421,7 @@ real_pread_pwrite: #endif err_prw: - - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } int @@ -476,10 +443,7 @@ io_args(int fd, void *mem, size_t nrw, return 0; err_io_args: - if (errno == saved_errno) - errno = EINVAL; - - return -1; + return set_errno(saved_errno, EINVAL); } int @@ -497,10 +461,7 @@ check_file(int fd, struct stat *st) return 0; err_is_file: - if (errno == saved_errno) - errno = EINVAL; - - return -1; + return set_errno(saved_errno, EINVAL); } /* POSIX can say whatever it wants. @@ -522,10 +483,7 @@ rw_over_nrw(ssize_t r, size_t nrw) return r; err_rw_over_nrw: - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } off_t @@ -582,73 +540,83 @@ try_err(int loop_err, int errval) void free_and_set_null(char **buf) { - if (buf == NULL || *buf == NULL) + if (buf == NULL) + err_exit(EFAULT, + "null ptr (to ptr for freeing) in free_and_set_null"); + + if (*buf == NULL) return; free(*buf); *buf = NULL; } -/* also returns error code */ -int -close_warn(int *fd, char *s) +void +open_on_eintr(const char *path, + int *fd, int flags, mode_t mode, + struct stat *st) { + int r = -1; int saved_errno = errno; - if (fd == NULL) { - if (s != NULL) - fprintf(stderr, "FAIL: %s: bad fd ptr\n", s); - return -1; - } + if (path == NULL) + err_exit(EINVAL, "open_on_eintr: null path"); - if (*fd < 0 && s != NULL) { - fprintf(stderr, "WARN: %s: already closed\n", s); - } else if (close(*fd) < 0) { - if (s != NULL) - fprintf(stderr, "FAIL: %s: close\n", s); - return -1; - } + if (fd == NULL) + err_exit(EFAULT, "%s: open_on_eintr: null fd ptr", path); - *fd = -1; - errno = saved_errno; + if (*fd >= 0) + err_exit(EBADF, "%s: open_on_eintr: file already open", path); - return 0; -} + do { + r = open(path, flags, mode); + } while (r == -1 && ( + errno == EINTR || errno == EAGAIN || + errno == EWOULDBLOCK || errno == ETXTBSY)); -/* TODO: remove this, and just check - * err on every close. */ -void -close_no_err(int *fd) -{ - int saved_errno = errno; + if (r < 0) + err_exit(errno, "%s: open_on_eintr: could not close", path); - if (fd == NULL || *fd < 0) - return; + *fd = r; - (void) close_on_eintr(*fd); - *fd = -1; + if (st != NULL) { + if (fstat(*fd, st) < 0) + err_exit(errno, "%s: stat", path); + + if (!S_ISREG(st->st_mode)) + err_exit(errno, "%s: not a regular file", path); + } + + if (lseek_on_eintr(*fd, 0, SEEK_CUR, 1, 1) == (off_t)-1) + err_exit(errno, "%s: file not seekable", path); errno = saved_errno; } -/* TODO: make fd a pointer insttead - and automatically reset -1 here */ -int -close_on_eintr(int fd) +void +close_on_eintr(int *fd) { int r; int saved_errno = errno; + if (fd == NULL) + err_exit(EINVAL, "close_on_eintr: null pointer"); + + if (*fd < 0) + return; + do { - r = close(fd); + r = close(*fd); } while (r == -1 && ( errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK || errno == ETXTBSY)); - if (r >= 0) - errno = saved_errno; + if (r < 0) + err_exit(errno, "close_on_eintr: could not close"); - return r; + *fd = -1; + + errno = saved_errno; } int @@ -708,8 +676,10 @@ rootfs(void) if (!fs_initialised) { - global_fs.rootfd = - open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC); + global_fs.rootfd = -1; + + open_on_eintr("/", &global_fs.rootfd, + O_RDONLY | O_DIRECTORY | O_CLOEXEC, 0400, NULL); if (global_fs.rootfd < 0) return NULL; @@ -721,6 +691,10 @@ rootfs(void) } /* filesystem sandboxing in userspace + * TODO: + missing length bound check. + potential CPU DoS on very long paths, spammed repeatedly. + perhaps cap at PATH_LEN? */ int fs_resolve_at(int dirfd, const char *path, int flags) @@ -761,7 +735,7 @@ fs_resolve_at(int dirfd, const char *path, int flags) /* close previous fd if not the original input */ if (curfd != dirfd) - (void) close_on_eintr(curfd); + close_on_eintr(&curfd); curfd = nextfd; nextfd = -1; @@ -774,16 +748,25 @@ err: saved_errno = errno; if (nextfd >= 0) - (void) close_on_eintr(nextfd); + close_on_eintr(&nextfd); /* close curfd only if it's not the original */ if (curfd != dirfd && curfd >= 0) - (void) close_on_eintr(curfd); + close_on_eintr(&curfd); errno = saved_errno; return -1; } +/* NOTE: + rejects . and .. but not empty strings + after normalisation. edge case: + ////// + + normalised implicitly, but might be good + to add a defensive check regardless. code + probably not exploitable in current state. + */ int fs_next_component(const char **p, char *name, size_t namesz) @@ -847,7 +830,7 @@ fs_open_component(int dirfd, const char *name, if (!S_ISDIR(st.st_mode)) { - (void) close_on_eintr(fd); + close_on_eintr(&fd); errno = ENOTDIR; return -1; } diff --git a/util/libreboot-utils/lib/io.c b/util/libreboot-utils/lib/io.c index 1f2064a0..eac6073e 100644 --- a/util/libreboot-utils/lib/io.c +++ b/util/libreboot-utils/lib/io.c @@ -4,6 +4,10 @@ * I/O functions specific to nvmutil. */ +/* TODO: local tmpfiles not being deleted + when flags==O_RDONLY e.g. dump command + */ + #include <sys/types.h> #include <sys/stat.h> @@ -27,21 +31,24 @@ open_gbe_file(void) int _flags; - xopen(&f->gbe_fd, f->fname, - cmd->flags | O_BINARY | - O_NOFOLLOW | O_CLOEXEC | O_NOCTTY, &f->gbe_st); + f->gbe_fd = -1; + + open_on_eintr(f->fname, &f->gbe_fd, + O_NOFOLLOW | O_CLOEXEC | O_NOCTTY, + ((cmd->flags & O_ACCMODE) == O_RDONLY) ? 0400 : 0600, + &f->gbe_st); if (f->gbe_st.st_nlink > 1) - b0rk(EINVAL, + err_exit(EINVAL, "%s: warning: file has multiple (%lu) hard links\n", f->fname, (size_t)f->gbe_st.st_nlink); if (f->gbe_st.st_nlink == 0) - b0rk(EIO, "%s: file unlinked while open", f->fname); + err_exit(EIO, "%s: file unlinked while open", f->fname); _flags = fcntl(f->gbe_fd, F_GETFL); if (_flags == -1) - b0rk(errno, "%s: fcntl(F_GETFL)", f->fname); + err_exit(errno, "%s: fcntl(F_GETFL)", f->fname); /* O_APPEND allows POSIX write() to ignore * the current write offset and write at EOF, @@ -49,7 +56,7 @@ open_gbe_file(void) */ if (_flags & O_APPEND) - b0rk(EIO, "%s: O_APPEND flag", f->fname); + err_exit(EIO, "%s: O_APPEND flag", f->fname); f->gbe_file_size = f->gbe_st.st_size; @@ -59,11 +66,14 @@ open_gbe_file(void) case SIZE_128KB: break; default: - b0rk(EINVAL, "File size must be 8KB, 16KB or 128KB"); + err_exit(EINVAL, "File size must be 8KB, 16KB or 128KB"); } +/* currently fails (EBADF), locks are advisory anyway: */ +/* if (lock_file(f->gbe_fd, cmd->flags) == -1) - b0rk(errno, "%s: can't lock", f->fname); + err_exit(errno, "%s: can't lock", f->fname); +*/ } void @@ -98,7 +108,7 @@ read_file(void) MAX_ZERO_RW_RETRY, OFF_ERR); if (_r < 0) - b0rk(errno, "%s: read failed", f->fname); + err_exit(errno, "%s: read failed", f->fname); /* copy to tmpfile */ @@ -107,34 +117,34 @@ read_file(void) MAX_ZERO_RW_RETRY, OFF_ERR); if (_r < 0) - b0rk(errno, "%s: %s: copy failed", + err_exit(errno, "%s: %s: copy failed", f->fname, f->tname); /* file size comparison */ if (fstat(f->tmp_fd, &_st) == -1) - b0rk(errno, "%s: stat", f->tname); + err_exit(errno, "%s: stat", f->tname); f->gbe_tmp_size = _st.st_size; if (f->gbe_tmp_size != f->gbe_file_size) - b0rk(EIO, "%s: %s: not the same size", + err_exit(EIO, "%s: %s: not the same size", f->fname, f->tname); /* needs sync, for verification */ if (fsync_on_eintr(f->tmp_fd) == -1) - b0rk(errno, "%s: fsync (tmpfile copy)", f->tname); + err_exit(errno, "%s: fsync (tmpfile copy)", f->tname); _r = rw_file_exact(f->tmp_fd, f->bufcmp, f->gbe_file_size, 0, IO_PREAD, NO_LOOP_EAGAIN, LOOP_EINTR, MAX_ZERO_RW_RETRY, OFF_ERR); if (_r < 0) - b0rk(errno, "%s: read failed (cmp)", f->tname); + err_exit(errno, "%s: read failed (cmp)", f->tname); if (memcmp(f->buf, f->bufcmp, f->gbe_file_size) != 0) - b0rk(errno, "%s: %s: read contents differ (pre-test)", + err_exit(errno, "%s: %s: read contents differ (pre-test)", f->fname, f->tname); } @@ -152,10 +162,10 @@ write_gbe_file(void) return; if (same_file(f->tmp_fd, &f->tmp_st, 0) < 0) - b0rk(errno, "%s: file inode/device changed", f->tname); + err_exit(errno, "%s: file inode/device changed", f->tname); if (same_file(f->gbe_fd, &f->gbe_st, 1) < 0) - b0rk(errno, "%s: file has changed", f->fname); + err_exit(errno, "%s: file has changed", f->fname); update_checksum = cmd->chksum_write; @@ -188,7 +198,7 @@ rw_gbe_file_part(size_t p, int rw_type, gbe_rw_size = cmd->rw_size; if (rw_type < IO_PREAD || rw_type > IO_PWRITE) - b0rk(errno, "%s: %s: part %lu: invalid rw_type, %d", + err_exit(errno, "%s: %s: part %lu: invalid rw_type, %d", f->fname, rw_type_str, (size_t)p, rw_type); mem_offset = gbe_mem_offset(p, rw_type_str); @@ -198,11 +208,11 @@ rw_gbe_file_part(size_t p, int rw_type, gbe_rw_size, file_offset, rw_type); if (rval == -1) - b0rk(errno, "%s: %s: part %lu", + err_exit(errno, "%s: %s: part %lu", f->fname, rw_type_str, (size_t)p); if ((size_t)rval != gbe_rw_size) - b0rk(EIO, "%s: partial %s: part %lu", + err_exit(EIO, "%s: partial %s: part %lu", f->fname, rw_type_str, (size_t)p); } @@ -226,7 +236,7 @@ write_to_gbe_bin(void) */ if (fsync_on_eintr(f->tmp_fd) == -1) - b0rk(errno, "%s: fsync (pre-verification)", + err_exit(errno, "%s: fsync (pre-verification)", f->tname); check_written_part(0); @@ -235,12 +245,12 @@ write_to_gbe_bin(void) report_io_err_rw(); if (f->io_err_gbe) - b0rk(EIO, "%s: bad write", f->fname); + err_exit(EIO, "%s: bad write", f->fname); saved_errno = errno; - f->io_err_gbe_bin |= -close_warn(&f->tmp_fd, f->tname); - f->io_err_gbe_bin |= -close_warn(&f->gbe_fd, f->fname); + close_on_eintr(&f->tmp_fd); + close_on_eintr(&f->gbe_fd); errno = saved_errno; @@ -307,10 +317,10 @@ check_written_part(size_t p) memset(f->pad, 0xff, sizeof(f->pad)); if (same_file(f->tmp_fd, &f->tmp_st, 0) < 0) - b0rk(errno, "%s: file inode/device changed", f->tname); + err_exit(errno, "%s: file inode/device changed", f->tname); if (same_file(f->gbe_fd, &f->gbe_st, 1) < 0) - b0rk(errno, "%s: file changed during write", f->fname); + err_exit(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); @@ -439,17 +449,8 @@ gbe_mv(void) tmp_gbe_bin_exists = 0; ret_gbe_mv: - - /* TODO: this whole section is bloat. - it can be generalised - */ - if (f->gbe_fd > -1) { - if (close_on_eintr(f->gbe_fd) < 0) { - f->gbe_fd = -1; - rval = -1; - } - f->gbe_fd = -1; + close_on_eintr(&f->gbe_fd); if (fsync_dir(f->fname) < 0) { f->io_err_gbe_bin = 1; @@ -457,13 +458,7 @@ ret_gbe_mv: } } - if (f->tmp_fd > -1) { - if (close_on_eintr(f->tmp_fd) < 0) { - f->tmp_fd = -1; - rval = -1; - } - f->tmp_fd = -1; - } + close_on_eintr(&f->tmp_fd); /* before this function is called, * tmp_fd may have been moved @@ -475,17 +470,12 @@ ret_gbe_mv: tmp_gbe_bin_exists = 0; } - if (rval < 0) { - /* if nothing set errno, - * we assume EIO, or we - * use what was set - */ - if (errno == saved_errno) - errno = EIO; - } else { - errno = saved_errno; - } + if (rval >= 0) + goto out; + return set_errno(saved_errno, EIO); +out: + errno = saved_errno; return rval; } @@ -540,11 +530,11 @@ gbe_x_offset(size_t p, const char *f_op, const char *d_type, off = ((off_t)p) * (off_t)nsize; if (off > ncmp - GBE_PART_SIZE) - b0rk(ECANCELED, "%s: GbE %s %s out of bounds", + err_exit(ECANCELED, "%s: GbE %s %s out of bounds", f->fname, d_type, f_op); if (off != 0 && off != ncmp >> 1) - b0rk(ECANCELED, "%s: GbE %s %s at bad offset", + err_exit(ECANCELED, "%s: GbE %s %s at bad offset", f->fname, d_type, f_op); return off; diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c index e499de34..61479b25 100644 --- a/util/libreboot-utils/lib/mkhtemp.c +++ b/util/libreboot-utils/lib/mkhtemp.c @@ -164,7 +164,7 @@ new_tmp_common(int *fd, char **path, int type, if (*fd < 0) goto err; - close_no_err(&dirfd); + close_on_eintr(&dirfd); errno = saved_errno; *path = dest; @@ -180,8 +180,8 @@ err: free_and_set_null(&dest); - close_no_err(&dirfd); - close_no_err(fd); + close_on_eintr(&dirfd); + close_on_eintr(fd); /* where a TMPDIR isn't found, and we err, * we pass this back through for the @@ -206,8 +206,8 @@ env_tmpdir(int bypass_all_sticky_checks, char **tmpdir, int allow_noworld_unsticky; int saved_errno = errno; - char tmp[] = "/tmp"; - char vartmp[] = "/var/tmp"; + static const char tmp[] = "/tmp"; + static const char vartmp[] = "/var/tmp"; /* tmpdir is a user override, if set */ if (override_tmpdir == NULL) @@ -238,26 +238,26 @@ env_tmpdir(int bypass_all_sticky_checks, char **tmpdir, allow_noworld_unsticky = 0; - if (world_writeable_and_sticky("/tmp", + if (world_writeable_and_sticky(tmp, allow_noworld_unsticky, bypass_all_sticky_checks)) { if (tmpdir != NULL) - *tmpdir = tmp; + *tmpdir = (char *)tmp; errno = saved_errno; - return "/tmp"; + return (char *)tmp; } - if (world_writeable_and_sticky("/var/tmp", + if (world_writeable_and_sticky(vartmp, allow_noworld_unsticky, bypass_all_sticky_checks)) { if (tmpdir != NULL) - *tmpdir = vartmp; + *tmpdir = (char *)vartmp; errno = saved_errno; - return "/var/tmp"; + return (char *)vartmp; } return NULL; @@ -295,11 +295,7 @@ tmpdir_policy(const char *path, return 0; err_tmpdir_policy: - - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } int @@ -331,11 +327,11 @@ same_dir(const char *a, const char *b) if (rval_scmp == 0) goto success_same_dir; - fd_a = fs_open(a, O_RDONLY | O_DIRECTORY); + fd_a = fs_open(a, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); if (fd_a < 0) goto err_same_dir; - fd_b = fs_open(b, O_RDONLY | O_DIRECTORY); + fd_b = fs_open(b, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); if (fd_b < 0) goto err_same_dir; @@ -348,8 +344,8 @@ same_dir(const char *a, const char *b) if (st_a.st_dev == st_b.st_dev && st_a.st_ino == st_b.st_ino) { - close_no_err(&fd_a); - close_no_err(&fd_b); + close_on_eintr(&fd_a); + close_on_eintr(&fd_b); success_same_dir: @@ -360,8 +356,8 @@ success_same_dir: return 1; } - close_no_err(&fd_a); - close_no_err(&fd_b); + close_on_eintr(&fd_a); + close_on_eintr(&fd_b); /* FAILURE (logical) */ @@ -374,13 +370,10 @@ err_same_dir: /* FAILURE (probably syscall) */ - close_no_err(&fd_a); - close_no_err(&fd_b); + close_on_eintr(&fd_a); + close_on_eintr(&fd_b); - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } /* bypass_all_sticky_checks: if set, @@ -467,20 +460,15 @@ world_writeable_and_sticky( goto sticky_hell; /* heaven visa denied */ sticky_heaven: - - close_no_err(&dirfd); + close_on_eintr(&dirfd); errno = saved_errno; return 1; sticky_hell: + close_on_eintr(&dirfd); - if (errno == saved_errno) - errno = EPERM; - - close_no_err(&dirfd); - errno = saved_errno; - + (void) set_errno(saved_errno, EPERM); return 0; } @@ -525,6 +513,17 @@ sticky_hell: * file ownership under hardened mode * (only lets you touch your own files/dirs) */ +/* + TODO: + some variables e.g. template vs suffix, + assumes they match. + we should test this explicitly, + but the way this is called is + currently safe - this would however + be nice for future library use + by outside projects. + this whole code needs to be reorganised +*/ int mkhtemp(int *fd, struct stat *st, @@ -607,7 +606,7 @@ mkhtemp(int *fd, errno = EEXIST; err: - close_no_err(fd); + close_on_eintr(fd); success: free_and_set_null(&fname_copy); @@ -733,7 +732,7 @@ mkhtemp_try_create(int dirfd, goto out; err: - close_no_err(fd); + close_on_eintr(fd); if (file_created) (void) unlinkat(dirfd, fname_copy, 0); @@ -828,11 +827,17 @@ err: if (linked) (void) unlinkat(dirfd, fname_copy, 0); - close_no_err(&tmpfd); + close_on_eintr(&tmpfd); return -1; } #endif +/* TODO: potential infinite loop under entropy failure. + * e.g. keeps returning low quality RNG, or atacker + * has control (DoS attack potential). + * possible solution: add a timeout (and abort if + * the timeout is reached) + */ int mkhtemp_fill_random(char *p, size_t xc) { @@ -912,6 +917,8 @@ int secure_file(int *fd, if (lock_file(*fd, flags) == -1) goto err_demons; + /* TODO: why would this be NULL? audit + * to find out. we should always verify! */ if (expected != NULL) if (fd_verify_identity(*fd, expected, &st_now) < 0) goto err_demons; @@ -924,11 +931,7 @@ int secure_file(int *fd, return 0; err_demons: - - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } int @@ -1038,9 +1041,5 @@ lock_file(int fd, int flags) return 0; err_lock_file: - - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } diff --git a/util/libreboot-utils/lib/num.c b/util/libreboot-utils/lib/num.c index f53f0cee..66fc26f1 100644 --- a/util/libreboot-utils/lib/num.c +++ b/util/libreboot-utils/lib/num.c @@ -51,6 +51,6 @@ void check_bin(size_t a, const char *a_name) { if (a > 1) - err_no_cleanup(0, EINVAL, "%s must be 0 or 1, but is %lu", + err_exit(EINVAL, "%s must be 0 or 1, but is %lu", a_name, (size_t)a); } diff --git a/util/libreboot-utils/lib/rand.c b/util/libreboot-utils/lib/rand.c index 863ace17..9edc8a5b 100644 --- a/util/libreboot-utils/lib/rand.c +++ b/util/libreboot-utils/lib/rand.c @@ -77,7 +77,7 @@ rsize(size_t n) { size_t rval = SIZE_MAX; if (!n) - err_no_cleanup(0, EFAULT, "rsize: division by zero"); + err_exit(EFAULT, "rsize: division by zero"); /* rejection sampling (clamp rand to eliminate modulo bias) */ for (; rval >= SIZE_MAX - (SIZE_MAX % n); rset(&rval, sizeof(rval))); @@ -92,13 +92,13 @@ mkrstr(size_t n) /* emulates spkmodem-decode */ size_t i; if (n == 0) - err_no_cleanup(0, EPERM, "mkrbuf: zero-byte request"); + err_exit(EPERM, "mkrbuf: zero-byte request"); if (n >= SIZE_MAX - 1) - err_no_cleanup(0, EOVERFLOW, "mkrbuf: overflow"); + err_exit(EOVERFLOW, "mkrbuf: overflow"); if (if_err((s = mkrbuf(n + 1)) == NULL, EFAULT)) - err_no_cleanup(0, EFAULT, "mkrstr: null"); + err_exit(EFAULT, "mkrstr: null"); for (i = 0; i < n; i++) while(*(s + i) == '\0') @@ -126,7 +126,7 @@ rset(void *buf, size_t n) goto err; if (n == 0) - err_no_cleanup(0, EPERM, "rset: zero-byte request"); + err_exit(EPERM, "rset: zero-byte request"); #if (defined(__OpenBSD__) || defined(__FreeBSD__) || \ defined(__NetBSD__) || defined(__APPLE__) || \ @@ -142,9 +142,7 @@ rset(void *buf, size_t n) #if defined(USE_URANDOM) && \ ((USE_URANDOM) > 0) int fd = -1; - - if ((fd = open("/dev/urandom", O_RDONLY)) < 0) - goto err; + open_on_eintr("/dev/urandom", &fd, O_RDONLY, 0400, NULL); retry_rand: if ((rc = read(fd, (unsigned char *)buf + off, n - off)) < 0) { #elif defined(__linux__) @@ -169,7 +167,7 @@ retry_rand: #if defined(USE_URANDOM) && \ ((USE_URANDOM) > 0) - close_no_err(&fd); + close_on_eintr(&fd); #endif goto out; #endif @@ -179,9 +177,9 @@ out: err: #if defined(USE_URANDOM) && \ ((USE_URANDOM) > 0) - close_no_err(&fd); + close_on_eintr(&fd); #endif - err_no_cleanup(0, ECANCELED, + err_exit(ECANCELED, "Randomisation failure, possibly unsupported in your kernel"); exit(EXIT_FAILURE); } diff --git a/util/libreboot-utils/lib/state.c b/util/libreboot-utils/lib/state.c index 41c851fb..a3cd5b1f 100644 --- a/util/libreboot-utils/lib/state.c +++ b/util/libreboot-utils/lib/state.c @@ -4,9 +4,6 @@ * State machine (singleton) for nvmutil data. */ -#ifdef __OpenBSD__ -#include <sys/param.h> -#endif #include <sys/types.h> #include <sys/stat.h> @@ -98,9 +95,9 @@ xstart(int argc, char *argv[]) return &us; if (argc < 3) - err_no_cleanup(0, EINVAL, "xstart: Too few arguments"); + err_exit(EINVAL, "xstart: Too few arguments"); if (argv == NULL) - err_no_cleanup(0, EINVAL, "xstart: NULL argv"); + err_exit(EINVAL, "xstart: NULL argv"); first_run = 0; @@ -113,41 +110,41 @@ xstart(int argc, char *argv[]) us.f.tname = NULL; if ((realdir = realpath(us.f.fname, NULL)) == NULL) - err_no_cleanup(0, errno, "xstart: can't get realpath of %s", + err_exit(errno, "xstart: can't get realpath of %s", us.f.fname); if (fs_dirname_basename(realdir, &dir, &base, 0) < 0) - err_no_cleanup(0, errno, "xstart: don't know CWD of %s", + err_exit(errno, "xstart: don't know CWD of %s", us.f.fname); if ((us.f.base = strdup(base)) == NULL) - err_no_cleanup(0, errno, "strdup base"); + err_exit(errno, "strdup base"); us.f.dirfd = fs_open(dir, O_RDONLY | O_DIRECTORY); if (us.f.dirfd < 0) - err_no_cleanup(0, errno, "%s: open dir", dir); + err_exit(errno, "%s: open dir", dir); if (new_tmpfile(&us.f.tmp_fd, &us.f.tname, dir, ".gbe.XXXXXXXXXX") < 0) - err_no_cleanup(0, errno, "%s", us.f.tname); + err_exit(errno, "%s", us.f.tname); if (fs_dirname_basename(us.f.tname, &tmpdir, &tmpbase_local, 0) < 0) - err_no_cleanup(0, errno, "tmp basename"); + err_exit(errno, "tmp basename"); us.f.tmpbase = strdup(tmpbase_local); if (us.f.tmpbase == NULL) - err_no_cleanup(0, errno, "strdup tmpbase"); + err_exit(errno, "strdup tmpbase"); free_and_set_null(&tmpdir); if (us.f.tname == NULL) - err_no_cleanup(0, errno, "x->f.tname null"); + err_exit(errno, "x->f.tname null"); if (*us.f.tname == '\0') - err_no_cleanup(0, errno, "x->f.tname empty"); + err_exit(errno, "x->f.tname empty"); if (fstat(us.f.tmp_fd, &us.f.tmp_st) < 0) - err_no_cleanup(0, errno, "%s: stat", us.f.tname); + err_exit(errno, "%s: stat", us.f.tname); memset(us.f.real_buf, 0, sizeof(us.f.real_buf)); memset(us.f.bufcmp, 0, sizeof(us.f.bufcmp)); @@ -164,70 +161,7 @@ xstatus(void) struct xstate *x = xstart(0, NULL); if (x == NULL) - err_no_cleanup(0, EACCES, "NULL pointer to xstate"); + err_exit(EACCES, "NULL pointer to xstate"); return x; } - -void -b0rk(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); -} - -int -exit_cleanup(void) -{ - struct xstate *x = xstatus(); - struct xfile *f; - - int close_err; - int saved_errno; - - close_err = 0; - saved_errno = errno; - - if (x != NULL) { - f = &x->f; - - close_no_err(&f->gbe_fd); - close_no_err(&f->tmp_fd); - close_no_err(&f->tmp_fd); - - if (f->tname != NULL) - if (unlink(f->tname) == -1) - close_err = 1; - - close_no_err(&f->dirfd); - free_and_set_null(&f->base); - free_and_set_null(&f->tmpbase); - } - - if (saved_errno) - errno = saved_errno; - - if (close_err) - return -1; - - return 0; -} diff --git a/util/libreboot-utils/lib/string.c b/util/libreboot-utils/lib/string.c index 986c7b8e..76141c58 100644 --- a/util/libreboot-utils/lib/string.c +++ b/util/libreboot-utils/lib/string.c @@ -40,9 +40,9 @@ vmalloc(void **buf, size_t size) void *rval = NULL; if (size >= SIZE_MAX - 1) - err_no_cleanup(0, EOVERFLOW, "integer overflow in vmalloc"); + err_exit(EOVERFLOW, "integer overflow in vmalloc"); if (buf == NULL) - err_no_cleanup(0, EFAULT, "Bad pointer passed to vmalloc"); + err_exit(EFAULT, "Bad pointer passed to vmalloc"); /* lots of programs will * re-initialise a buffer @@ -52,14 +52,14 @@ vmalloc(void **buf, size_t size) * force the programmer to behave */ if (*buf != NULL) - err_no_cleanup(0, EFAULT, "Non-null pointer given to vmalloc"); + err_exit(EFAULT, "Non-null pointer given to vmalloc"); if (!size) - err_no_cleanup(0, EFAULT, + err_exit(EFAULT, "Tried to vmalloc(0) and that is very bad. Fix it now"); if ((rval = malloc(size)) == NULL) - err_no_cleanup(0, errno, "malloc fail in vmalloc"); + err_exit(errno, "malloc fail in vmalloc"); return *buf = rval; } @@ -201,14 +201,10 @@ scatn(ssize_t sc, const char **sv, errno = saved_errno; return 0; err: - if (ct != NULL) - free(ct); - if (size != NULL) - free(size); - if (errno == saved_errno) - errno = EFAULT; + free_and_set_null(&ct); + free_and_set_null((char **)&size); - return -1; + return set_errno(saved_errno, EFAULT); } /* strict strcat */ @@ -285,21 +281,32 @@ err: return -1; } +/* on functions that return with errno, + * i sometimes have a default fallback, + * which is set if errno wasn't changed, + * under error condition. + */ +int +set_errno(int saved_errno, int fallback) +{ + if (errno == saved_errno) + errno = fallback; + return -1; +} + /* the one for nvmutil state is in state.c */ /* this one just exits */ void -err_no_cleanup(int stfu, int nvm_errval, const char *msg, ...) +err_exit(int nvm_errval, const char *msg, ...) { va_list args; int saved_errno = errno; const char *p; -#if defined(__OpenBSD__) && defined(OpenBSD) -#if (OpenBSD) >= 509 - if (pledge("stdio", NULL) == -1) - fprintf(stderr, "pledge failure during exit"); -#endif -#endif + func_t err_cleanup = errhook(NULL); + err_cleanup(); + errno = saved_errno; + if (!errno) saved_errno = errno = ECANCELED; @@ -318,6 +325,37 @@ err_no_cleanup(int stfu, int nvm_errval, const char *msg, ...) exit(EXIT_FAILURE); } +/* the err function will + * call this upon exit, and + * cleanup will be performed + * e.g. you might want to + * close some files, depending + * on your program. + * see: err_exit() + */ +func_t errhook(func_t ptr) +{ + static int set = 0; + static func_t hook = NULL; + + if (!set) { + set = 1; + + if (ptr == NULL) + hook = no_op; + else + hook = ptr; + } + + return hook; +} + +void +no_op(void) +{ + return; +} + const char * getnvmprogname(void) { @@ -363,6 +401,8 @@ lbgetprogname(char *argv0) return progname; } +/* https://man.openbsd.org/pledge.2 + https://man.openbsd.org/unveil.2 */ int xpledgex(const char *promises, const char *execpromises) { @@ -370,12 +410,11 @@ xpledgex(const char *promises, const char *execpromises) (void) promises, (void) execpromises, (void) saved_errno; #ifdef __OpenBSD__ if (pledge(promises, execpromises) == -1) - err_no_cleanup(0, errno, "pledge"); + err_exit(errno, "pledge"); #endif errno = saved_errno; return 0; } - int xunveilx(const char *path, const char *permissions) { @@ -383,7 +422,7 @@ xunveilx(const char *path, const char *permissions) (void) path, (void) permissions, (void) saved_errno; #ifdef __OpenBSD__ if (pledge(promises, execpromises) == -1) - err_no_cleanup(0, errno, "pledge"); + err_exit(errno, "pledge"); #endif errno = saved_errno; return 0; diff --git a/util/libreboot-utils/lib/usage.c b/util/libreboot-utils/lib/usage.c index 2b5a93ca..ebec119e 100644 --- a/util/libreboot-utils/lib/usage.c +++ b/util/libreboot-utils/lib/usage.c @@ -26,5 +26,5 @@ usage(void) util, util, util, util, util, util, util); - b0rk(EINVAL, "Too few arguments"); + err_exit(EINVAL, "Too few arguments"); } diff --git a/util/libreboot-utils/lib/word.c b/util/libreboot-utils/lib/word.c index 6563e67a..85e1d88b 100644 --- a/util/libreboot-utils/lib/word.c +++ b/util/libreboot-utils/lib/word.c @@ -63,6 +63,6 @@ check_nvm_bound(size_t c, size_t p) check_bin(p, "part number"); if (c >= NVM_WORDS) - b0rk(ECANCELED, "check_nvm_bound: out of bounds %lu", + err_exit(ECANCELED, "check_nvm_bound: out of bounds %lu", (size_t)c); } |