diff options
Diffstat (limited to 'util/libreboot-utils/lib')
| -rw-r--r-- | util/libreboot-utils/lib/file.c | 103 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/mkhtemp.c | 52 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/num.c | 41 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/rand.c | 43 |
4 files changed, 46 insertions, 193 deletions
diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c index 552618d6..5fdef7b3 100644 --- a/util/libreboot-utils/lib/file.c +++ b/util/libreboot-utils/lib/file.c @@ -69,29 +69,6 @@ err_same_file: return -1; } -/* open() but with abort traps - */ -/* TODO: also support other things here than files. - and then use, throughout the program. - in particular, use of openat might help - (split the path) - (see: link attack mitigations throughout nvmutil) - - make it return, and handle the return value/errno - - (this could return e.g. EINTR) - - TODO: this function is not used by mkhtemp, nor will - it probably be, it's currently used by nvmutil, - for opening intel gbe nvm config files. i can - probably remove it though and unify witth some - of the verification code now used for mkhtemp - -TODO: and don't abort. return -1. and handle in the caller. - -minor obstacle: the mkhtemp code always requires absolute -paths, whereas the gbe editor takes relative paths. - */ void xopen(int *fd_ptr, const char *path, int flags, struct stat *st) { @@ -108,10 +85,6 @@ xopen(int *fd_ptr, const char *path, int flags, struct stat *st) err_no_cleanup(0, errno, "%s: file not seekable", path); } -/* fsync() the directory of a file, - * useful for atomic writes - */ - int fsync_dir(const char *path) { @@ -196,19 +169,6 @@ err_fsync_dir: return -1; } -/* - * Safe I/O functions wrapping around - * read(), write() and providing a portable - * analog of both pread() and pwrite(). - * These functions are designed for maximum - * robustness, checking NULL inputs, overflowed - * outputs, and all kinds of errors that the - * standard libc functions don't. - * - * Looping on EINTR and EAGAIN is supported. - * EINTR/EAGAIN looping is done indefinitely. - */ - /* rw_file_exact() - Read perfectly or die * * Read/write, and absolutely insist on an @@ -243,6 +203,7 @@ rw_file_exact(int fd, unsigned char *mem, size_t nrw, size_t retries_on_zero; int saved_errno = errno; + errno = 0; rval = 0; @@ -317,12 +278,6 @@ err_rw_file_exact: /* prw() - portable read-write with more * safety checks than barebones libc * - * portable pwrite/pread on request, or real - * pwrite/pread libc functions can be used. - * the portable (non-libc) pread/pwrite is not - * thread-safe, because it does not prevent or - * mitigate race conditions on file descriptors - * * If you need real pwrite/pread, just compile * with flag: REAL_POS_IO=1 * @@ -340,6 +295,11 @@ err_rw_file_exact: * a change was detected, assuming * nothing else is touching it now * off_reset 0: never reset if changed + * + * REAL_POS_IO is enabled by default in common.h + * and the fallback version was written for fun. + * You should just use the real one (REAL_POS_IO 1), + * since it is generally more reliable. */ ssize_t @@ -359,6 +319,7 @@ prw(int fd, void *mem, size_t nrw, off_t off_last; #endif int saved_errno = errno; + errno = 0; if (io_args(fd, mem, nrw, off, rw_type) == -1) @@ -568,8 +529,6 @@ err_rw_over_nrw: return -1; } -#if !defined(REAL_POS_IO) || \ - REAL_POS_IO < 1 off_t lseek_on_eintr(int fd, off_t off, int whence, int loop_eagain, int loop_eintr) @@ -588,10 +547,9 @@ lseek_on_eintr(int fd, off_t off, int whence, return old; } -#endif /* two functions that reduce sloccount by - * two hundred lines... no, now three. */ + * two hundred lines */ int if_err(int condition, int errval) { @@ -603,13 +561,6 @@ if_err(int condition, int errval) return 1; } -/* technically pointless, but stylistically - * pleasing alongside if_err chains. - * use this one for syscalls that are - * expected to set errno - * also use it for non-system calls - * that act like them, e.g. prw() or - * rw_write_exact() */ int if_err_sys(int condition) { @@ -665,10 +616,8 @@ close_warn(int *fd, char *s) return 0; } -/* TODO: remove this. giant liability. - make close calls always err instead, - when they fail. otherwise we hide bugs! - */ +/* TODO: remove this, and just check + * err on every close. */ void close_no_err(int *fd) { @@ -685,7 +634,6 @@ close_no_err(int *fd) /* TODO: make fd a pointer insttead and automatically reset -1 here */ -/* BUT DO NOT reset -1 on error */ int close_on_eintr(int fd) { @@ -732,11 +680,10 @@ fs_rename_at(int olddirfd, const char *old, return renameat(olddirfd, old, newdirfd, new); } -/* secure open, based on - * relative path to root +/* secure open, based on relative path to root * - * always a fixed fd for / - * see: rootfs() + * always a fixed fd for / see: rootfs() + * and fs_resolve_at() */ int fs_open(const char *path, int flags) @@ -751,12 +698,8 @@ fs_open(const char *path, int flags) return fs_resolve_at(fs->rootfd, path + 1, flags); } -/* singleton function - * that returns a fixed - * descriptor of / - * - * used throughout, for - * repeated integrity checks +/* singleton function that returns a fixed descriptor of / + * used throughout, for repeated integrity checks */ struct filesystem * rootfs(void) @@ -778,8 +721,7 @@ rootfs(void) return &global_fs; } -/* filesystem sandboxing. - * (in userspace) +/* filesystem sandboxing in userspace */ int fs_resolve_at(int dirfd, const char *path, int flags) @@ -818,10 +760,9 @@ fs_resolve_at(int dirfd, const char *path, int flags) if (nextfd < 0) goto err; - /* close previous fd IF it is not the original input */ - if (curfd != dirfd) { + /* close previous fd if not the original input */ + if (curfd != dirfd) (void) close_on_eintr(curfd); - } curfd = nextfd; nextfd = -1; @@ -899,8 +840,6 @@ fs_open_component(int dirfd, const char *name, (is_last ? flags : (O_RDONLY | O_DIRECTORY)) | O_NOFOLLOW | O_CLOEXEC, (flags & O_CREAT) ? 0600 : 0); - /* the patient always lies - */ if (!is_last) { if (if_err(fd < 0, EBADF) || @@ -972,9 +911,6 @@ fs_dirname_basename(const char *path, /* portable wrapper for use of openat2 on linux, * with fallback for others e.g. openbsd - * - * BONUS: arg checks - * TODO: consider EINTR/EAGAIN retry loop */ int openat2p(int dirfd, const char *path, @@ -1025,8 +961,7 @@ retry: } int -mkdirat_on_eintr( /* <-- say that 10 times to please the demon */ - int dirfd, +mkdirat_on_eintr(int dirfd, const char *path, mode_t mode) { int saved_errno = errno; diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c index a669e208..0e0169e4 100644 --- a/util/libreboot-utils/lib/mkhtemp.c +++ b/util/libreboot-utils/lib/mkhtemp.c @@ -51,22 +51,6 @@ new_tmpdir(int *fd, char **path, char *tmpdir, tmpdir, template); } -/* note: tmpdir is an override of TMPDIR or /tmp or /var/tmp */ -/* WARNING: - * on error, *path (at **path) may be - NULL, or if the error pertains to - an actual TMPDIR, set. if set, it - will be using *static* memory and - must not be freed. on success, - a pointer to heap memory is set - instead. - * see: - * env_tmpdir() - * this is for error reports if e.g. - * TMPDIR isn't found (but is set) - * if TMPDIR isn't set, it will - * default to /tmp or /var/tmp - */ int new_tmp_common(int *fd, char **path, int type, char *tmpdir, const char *template) @@ -443,20 +427,8 @@ world_writeable_and_sticky( goto sticky_hell; } - /* must be fully executable - * by everyone, or openat2 - * becomes unreliable** - * - * TODO: loosen these, as a toggle. - * execution rights isn't - * really a requirement for - * TMPDIR, except maybe search, - * but this function will be - * generalised at some point - * for use in other tools - * besides just mkhtemp. - */ - /* + /* all of these checks are probably + * redundant (execution rights) if (!(st.st_mode & S_IXUSR) || !(st.st_mode & S_IXGRP) || !(st.st_mode & S_IXOTH)) { @@ -473,7 +445,7 @@ world_writeable_and_sticky( if (bypass_all_sticky_checks) goto sticky_heaven; /* normal == no security */ - /* unhinged leah mode: + /* extremely not-libc mode: */ if (st.st_mode & S_IWOTH) { /* world writeable */ @@ -488,9 +460,7 @@ world_writeable_and_sticky( goto sticky_hell; /* not sticky */ } - /* if anyone even looks at you funny, drop - * everything on the floor and refuse to function - */ + /* for good measure */ if (faccessat(dirfd, ".", X_OK, AT_EACCESS) < 0) goto sticky_hell; @@ -503,7 +473,6 @@ world_writeable_and_sticky( goto sticky_hell; /* heaven visa denied */ sticky_heaven: -/* i like the one in hamburg better */ close_no_err(&dirfd); errno = saved_errno; @@ -515,10 +484,7 @@ sticky_hell: if (errno == saved_errno) errno = EPERM; - saved_errno = errno; - close_no_err(&dirfd); - errno = saved_errno; return 0; @@ -909,11 +875,9 @@ retry_rand: /* WARNING: **ONCE** per file. * - * !!! DO NOT RUN TWICE PER FILE. BEWARE OF THE DEMON !!! - * watch out for spikes! - */ -/* TODO: bad_flags can be negative, and is - * ignored if it is. should we err instead? + * some of these checks will trip up + * if you do them twice; all of them + * only need to be done once anyway. */ int secure_file(int *fd, struct stat *st, @@ -945,7 +909,7 @@ int secure_file(int *fd, if (check_seek) { /***********/ if (lseek(*fd, 0, SEEK_CUR) == (off_t)-1) goto err_demons; - } /* don't release the demon */ + } /* don't release the demon! */ if (if_err(st->st_nlink != 1, ELOOP) || if_err(st->st_uid != geteuid() && geteuid() != 0, EPERM) || diff --git a/util/libreboot-utils/lib/num.c b/util/libreboot-utils/lib/num.c index 92710c35..79d6b409 100644 --- a/util/libreboot-utils/lib/num.c +++ b/util/libreboot-utils/lib/num.c @@ -1,12 +1,8 @@ /* SPDX-License-Identifier: MIT * Copyright (c) 2026 Leah Rowe <leah@libreboot.org> * - * Numerical functions. - * NOTE: randomness was moved to rand.c - */ - -/* -TODO: properly handle errno in this file + * Non-randomisation-related numerical functions. + * For rand functions, see: rand.c */ #ifdef __OpenBSD__ @@ -27,42 +23,11 @@ TODO: properly handle errno in this file #include "../include/common.h" -/* TODO: - * make this and errno handling more - * flexible - - in particular: - hextonum could be modified to - write into a buffer instead, - with the converted numbers, - of an arbitrary length - */ unsigned short hextonum(char ch_s) { int saved_errno = errno; - /* rlong() can return error, - but preserves errno if no - error. we need to detect - this because it handles - /dev/urandom sometimes - - therefore, if it's zero - at start, we know if there - was an err at the end, by - return value zero, if errno - was set; this is technically - valid, since zero is also - a valid random number! - - it's an edge case that i had - to fix. i'll rewrite the code - better later. for now, it - should be ok. - */ - errno = 0; - unsigned char ch; size_t rval; @@ -85,8 +50,6 @@ hextonum(char ch_s) if (ch == '?' || ch == 'x') { rset(&rval, sizeof(rval)); - if (errno > 0) - goto err_hextonum; goto hextonum_success; } diff --git a/util/libreboot-utils/lib/rand.c b/util/libreboot-utils/lib/rand.c index ac94a482..3155eec3 100644 --- a/util/libreboot-utils/lib/rand.c +++ b/util/libreboot-utils/lib/rand.c @@ -20,6 +20,9 @@ #if defined(USE_URANDOM) && \ ((USE_URANDOM) > 0) #include <fcntl.h> /* if not arc4random: /dev/urandom */ +#elif defined(__linux__) +#include <sys/random.h> +#include <sys/syscall.h> #endif #include <fcntl.h> @@ -69,35 +72,20 @@ * or your program dies. */ -int -win_lottery(void) /* are u lucky? */ +void * +rmalloc(size_t *rval) { - size_t size = 0; - int rval; - - char *s1 = rmalloc(&size); - char *s2 = rmalloc(&size); - - if (scmp(s1, s2, BUFSIZ + 2, &rval) >= 0 && - rval == 0) - rval = 1; /* winner! */ - else - rval = 0; - - free_if_null(&s1); - free_if_null(&s2); - - return rval; + return if_err(rval == NULL, EFAULT) ? + NULL : mkrstr(*rval = rsize(BUFSIZ)); } -void * -rmalloc(size_t *rval) +size_t +rsize(size_t n) { - if (if_err(rval == NULL, EFAULT)) - return NULL; + size_t rval = SIZE_MAX; + for (; rval >= SIZE_MAX - (SIZE_MAX % n); rset(&rval, sizeof(rval))); - rset(rval, sizeof(*rval)); - return mkrstr(*rval %= BUFSIZ); + return rval % n; } char * @@ -109,7 +97,7 @@ mkrstr(size_t n) /* emulates spkmodem-decode */ if (n == 0) err_no_cleanup(0, EPERM, "mkrbuf: zero-byte request"); - if (n == SIZE_MAX) + if (n >= SIZE_MAX - 1) err_no_cleanup(0, EOVERFLOW, "mkrbuf: overflow"); if (if_err((s = mkrbuf(n + 1)) == NULL, EFAULT)) @@ -127,11 +115,14 @@ mkrstr(size_t n) /* emulates spkmodem-decode */ void * mkrbuf(size_t n) { - void *buf; + void *buf = ""; if (n == 0) err_no_cleanup(0, EPERM, "mkrbuf: zero-byte request"); + if (n >= SIZE_MAX - 1) + err_no_cleanup(0, EOVERFLOW, "integer overflow in mkrbuf"); + if ((buf = malloc(n)) == NULL) err_no_cleanup(0, ENOMEM, "mkrbuf: malloc"); |