diff options
Diffstat (limited to 'util')
| -rw-r--r-- | util/libreboot-utils/lib/file.c | 50 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/mkhtemp.c | 52 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/num.c | 40 |
3 files changed, 27 insertions, 115 deletions
diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c index d13d9d93..63a2de16 100644 --- a/util/libreboot-utils/lib/file.c +++ b/util/libreboot-utils/lib/file.c @@ -294,6 +294,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 @@ -542,7 +547,7 @@ lseek_on_eintr(int fd, off_t off, int whence, } /* two functions that reduce sloccount by - * two hundred lines... no, now three. */ + * two hundred lines */ int if_err(int condition, int errval) { @@ -554,13 +559,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) { @@ -616,10 +614,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) { @@ -636,7 +632,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) { @@ -683,11 +678,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) @@ -702,12 +696,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) @@ -729,8 +719,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) @@ -769,10 +758,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; @@ -921,9 +909,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, @@ -974,8 +959,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..faf06121 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; @@ -95,7 +60,6 @@ hextonum(char ch_s) hextonum_success: - errno = saved_errno; return (unsigned short)rval & 0xf; err_hextonum: |