libreboot-utils: stricter errno handling

where possible, try not to clobber sys errno. override
it only when relatively safe.

also: when a syscall succeeds, it may set errno. this
is rare, but permitted (nothing specified against it
in specs, and the specs say that errno is undefined
on success).

i'm not libc, but i'm wrapping around it, so i need
to be careful in how i handle the errno value.

also:

i removed the requirement for directories to be
executable, in mkhtemp.c, because this isn't required
and will only break certain setups.

in world_writeable and sticky, i made the checks stricter:
the faccessat check was being skipped on some paths, so
i've closed that loophole now.

i also generally cleaned up some code, as part of the errno
handling refactoring, where it made sense to do so, plus a
few other bits of code cleanup.

Signed-off-by: Leah Rowe <leah@libreboot.org>

1 files changed, 133 insertions, 155 deletions

diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c
index 42c91843..805db726 100644
--- a/util/libreboot-utils/lib/file.c
+++ b/util/libreboot-utils/lib/file.c
@@ -44,51 +44,31 @@ same_file(int fd, struct stat *st_old,
 {
 	struct stat st;
 	int saved_errno = errno;
+	int rval = 0;
+	errno = 0;
 
-	/* TODO: null/-1 checks
-	 *       like this can be
-	 *       generalised
-	 */
-	if (st_old == NULL) {
-		errno = EFAULT;
-		goto err_same_file;
-	}
-	if (fd < 0) {
-		errno = EBADF;
-		goto err_same_file;
-	}
-
-	if (fstat(fd, &st) == -1)
-		goto err_same_file;
-
-	if (fd_verify_regular(fd, st_old, &st) < 0)
-		goto err_same_file;
-
-	if (check_size &&
-	    st.st_size != st_old->st_size)
-		goto err_same_file;
+	if (if_err(st_old == NULL, EFAULT) ||
+	    if_err(fd < 0, EBADF) ||
+	    (rval = fstat(fd, &st)) < 0 ||
+	    (rval = fd_verify_regular(fd, st_old, &st)) < 0 ||
+	    if_err(check_size && st.st_size != st_old->st_size, ESTALE))
+		return with_fallback_errno(ESTALE);
 
-	errno = saved_errno;
+	reset_caller_errno(rval);
 	return 0;
-
-err_same_file:
-	return set_errno(saved_errno, ESTALE);
 }
 
 int
 fsync_dir(const char *path)
 {
 	int saved_errno = errno;
-
 	size_t pathlen = 0;
-
 	char *dirbuf = NULL;
 	int dirfd = -1;
-
 	char *slash = NULL;
 	struct stat st = {0};
-
-	int close_errno;
+	int rval = 0;
+	errno = 0;
 
 	if (if_err(slen(path, PATH_MAX, &pathlen) == 0, EINVAL))
 		goto err_fsync_dir;
@@ -119,26 +99,23 @@ fsync_dir(const char *path)
 );
 
 	if (if_err_sys(dirfd < 0) ||
-	    if_err_sys(fstat(dirfd, &st) < 0) ||
+	    if_err_sys((rval = fstat(dirfd, &st)) < 0) ||
 	    if_err(!S_ISDIR(st.st_mode), ENOTDIR)
 	    ||
-	    if_err_sys(fsync_on_eintr(dirfd) == -1))
+	    if_err_sys((rval = fsync_on_eintr(dirfd)) == -1))
 		goto err_fsync_dir;
 
 	close_on_eintr(&dirfd);
-
 	free_and_set_null(&dirbuf);
 
-	errno = saved_errno;
+	reset_caller_errno(rval);
 	return 0;
 
 err_fsync_dir:
-
-
 	free_and_set_null(&dirbuf);
 	close_on_eintr(&dirfd);
 
-	return set_errno(saved_errno, EIO);
+	return with_fallback_errno(EIO);
 }
 
 /* rw_file_exact() - Read perfectly or die
@@ -162,34 +139,23 @@ ssize_t
 rw_file_exact(int fd, unsigned char *mem, size_t nrw,
     off_t off, int rw_type, size_t max_retries, int off_reset)
 {
-	ssize_t rval;
-	ssize_t rc;
-
+	int saved_errno = errno;
+	ssize_t rval = 0;
+	ssize_t rc = 0;
 	size_t nrw_cur;
-
 	off_t off_cur;
 	void *mem_cur;
-
-	size_t retries_on_zero;
-
-	int saved_errno = errno;
+	size_t retries_on_zero = 0;
 	errno = 0;
 
-	rval = 0;
-
-	rc = 0;
-	retries_on_zero = 0;
-
 	if (io_args(fd, mem, nrw, off, rw_type) == -1)
 		goto err_rw_file_exact;
 
 	while (1) {
 	
 		/* Prevent theoretical overflow */
-		if (rval >= 0 && (size_t)rval > (nrw - rc)) {
-			errno = EOVERFLOW;
+		if (if_err(rval >= 0 && (size_t)rval > (nrw - rc), EOVERFLOW))
 			goto err_rw_file_exact;
-		}
 
 		rc += rval;
 		if ((size_t)rc >= nrw)
@@ -198,50 +164,36 @@ rw_file_exact(int fd, unsigned char *mem, size_t nrw,
 		mem_cur = (void *)(mem + (size_t)rc);
 		nrw_cur = (size_t)(nrw - (size_t)rc);
 
-		if (off < 0) {
-			errno = EOVERFLOW;
+		if (if_err(off < 0, EOVERFLOW))
 			goto err_rw_file_exact;
-		}
 
 		off_cur = off + (off_t)rc;
 
-		rval = prw(fd, mem_cur, nrw_cur, off_cur,
-		    rw_type);
-
-		if (rval < 0)
+		if ((rval = rw(fd, mem_cur, nrw_cur, off_cur, rw_type)) < 0)
 			goto err_rw_file_exact;
 
 		if (rval == 0) {
 			if (retries_on_zero++ < max_retries)
 				continue;
 
-			errno = EIO;
 			goto err_rw_file_exact;
 		}
 
 		retries_on_zero = 0;
 	}
 
-	if ((size_t)rc != nrw) {
-
-		errno = EIO;
-		goto err_rw_file_exact;
-	}
-
-	rval = rw_over_nrw(rc, nrw);
-	if (rval < 0)
+	if (if_err((size_t)rc != nrw, EIO) ||
+	    (rval = rw_over_nrw(rc, nrw)) < 0)
 		goto err_rw_file_exact;
 
-	errno = saved_errno;
-
+	reset_caller_errno(rval);
 	return rval;
 
 err_rw_file_exact:
-
-	return set_errno(saved_errno, EIO);
+	return with_fallback_errno(EIO);
 }
 
-/* prw() - portable read-write with more
+/* rw() - read-write but with more
  * safety checks than barebones libc
  *
  * A fallback is provided for regular read/write.
@@ -250,35 +202,40 @@ err_rw_file_exact:
  */
 
 ssize_t
-prw(int fd, void *mem, size_t nrw,
+rw(int fd, void *mem, size_t nrw,
     off_t off, int rw_type)
 {
-	ssize_t rval;
-	ssize_t r;
+	ssize_t rval = 0;
+	ssize_t r = -1;
 	struct stat st;
 	int saved_errno = errno;
 	errno = 0;
 
 	if (io_args(fd, mem, nrw, off, rw_type) == -1)
-		return set_errno(saved_errno, EIO);
-
-	r = -1;
+		return with_fallback_errno(EINVAL);
 
-	if (rw_type == IO_WRITE)
+	switch (rw_type) {
+	case IO_WRITE:
 		r = write_on_eintr(fd, mem, nrw);
-	else if (rw_type == IO_READ)
+		break;
+	case IO_READ:
 		r = read_on_eintr(fd, mem, nrw);
-	else if (rw_type == IO_PWRITE)
+		break;
+	case IO_PWRITE:
 		r = pwrite_on_eintr(fd, mem, nrw, off);
-	else if (rw_type == IO_PREAD)
+		break;
+	case IO_PREAD:
 		r = pread_on_eintr(fd, mem, nrw, off);
+		break;
+	default:
+		errno = EINVAL;
+		break;
+	}
 
 	if ((rval = rw_over_nrw(r, nrw)) < 0)
-		return set_errno(saved_errno, EIO);
-
-	if (rval >= 0 && !errno)
-		errno = saved_errno;
+		return with_fallback_errno(EIO);
 
+	reset_caller_errno(rval);
 	return rval;
 }
 
@@ -287,6 +244,7 @@ io_args(int fd, void *mem, size_t nrw,
     off_t off, int rw_type)
 {
 	int saved_errno = errno;
+	errno = 0;
 
 	if (if_err(mem == NULL, EFAULT) ||
 	    if_err(fd < 0, EBADF) ||
@@ -297,29 +255,31 @@ io_args(int fd, void *mem, size_t nrw,
 	    if_err(rw_type > IO_PWRITE, EINVAL))
 		goto err_io_args;
 
-	errno = saved_errno;
+	reset_caller_errno(0);
 	return 0;
 
 err_io_args:
-	return set_errno(saved_errno, EINVAL);
+	return with_fallback_errno(EINVAL);
 }
 
 int
 check_file(int fd, struct stat *st)
 {
 	int saved_errno = errno;
+	int rval = 0;
+	errno = 0;
 
 	if (if_err(fd < 0, EBADF) ||
 	    if_err(st == NULL, EFAULT) ||
-	    if_err(fstat(fd, st) == -1, 0) ||
+	    ((rval = fstat(fd, st)) == -1) ||
 	    if_err(!S_ISREG(st->st_mode), EBADF))
 		goto err_is_file;
 
-	errno = saved_errno;
+	reset_caller_errno(rval);
 	return 0;
 
 err_is_file:
-	return set_errno(saved_errno, EINVAL);
+	return with_fallback_errno(EINVAL);
 }
 
 /* POSIX can say whatever it wants.
@@ -328,15 +288,12 @@ err_is_file:
 ssize_t
 rw_over_nrw(ssize_t r, size_t nrw)
 {
-	int saved_errno = errno;
-
-	if (if_err(!nrw, 0) ||
-	    if_err(r == -1, 0) ||
+	if (if_err(!nrw, EIO) ||
+	    (r == -1) ||
 	    if_err((size_t)r > SSIZE_MAX, ERANGE) ||
 	    if_err((size_t)r > nrw, ERANGE))
-		return set_errno(saved_errno, EIO);
+		return with_fallback_errno(EIO);
 
-	errno = saved_errno;
 	return r;
 }
 
@@ -347,10 +304,8 @@ if_err(int condition, int errval)
 {
 	if (!condition)
 		return 0;
-
 	if (errval)
 		errno = errval;
-
 	return 1;
 }
 int
@@ -431,6 +386,7 @@ fs_resolve_at(int dirfd, const char *path, int flags)
 	int saved_errno = errno;
 	int r;
 	int is_last;
+	errno = 0;
 
 	if (dirfd < 0 || path == NULL || *path == '\0') {
 		errno = EINVAL;
@@ -461,7 +417,7 @@ fs_resolve_at(int dirfd, const char *path, int flags)
 		nextfd = -1;
 	}
 
-	errno = saved_errno;
+	reset_caller_errno(0);
 	return curfd;
 
 err:
@@ -475,7 +431,7 @@ err:
 		close_on_eintr(&curfd);
 
 	errno = saved_errno;
-	return -1;
+	return with_fallback_errno(EIO);
 }
 
 /* NOTE:
@@ -515,41 +471,36 @@ fs_next_component(const char **p,
 	name[len] = '\0';
 
 	/* reject . and .. */
-	if ((name[0] == '.' && name[1] == '\0') ||
-	    (name[0] == '.' && name[1] == '.' && name[2] == '\0')) {
-		errno = EPERM;
-		return -1;
-	}
+	if (if_err((name[0] == '.' && name[1] == '\0') ||
+	    (name[0] == '.' && name[1] == '.' && name[2] == '\0'), EPERM))
+		goto err;
 
 	*p = s + len;
 	return 1;
+err:
+	return with_fallback_errno(EPERM);
 }
 
 int
 fs_open_component(int dirfd, const char *name,
     int flags, int is_last)
 {
+	int saved_errno = errno;
 	int fd;
 	struct stat st;
+	errno = 0;
 
 	fd = openat_on_eintr(dirfd, name,
 	    (is_last ? flags : (O_RDONLY | O_DIRECTORY)) |
 	    O_NOFOLLOW | O_CLOEXEC, (flags & O_CREAT) ? 0600 : 0);
 
-	if (!is_last) {
-
-		if (if_err(fd < 0, EBADF) ||
-		    if_err_sys(fstat(fd, &st) < 0))
-			return -1;
-
-		if (!S_ISDIR(st.st_mode)) {
-
-			close_on_eintr(&fd);
-			errno = ENOTDIR;
-			return -1;
-		}
-	}
+	if (!is_last &&
+	    (if_err(fd < 0, EBADF) ||
+	     if_err_sys(fstat(fd, &st) < 0) ||
+	     if_err(!S_ISDIR(st.st_mode), ENOTDIR)))
+		return with_fallback_errno(EIO);
 
+	reset_caller_errno(fd);
 	return fd;
 }
 
@@ -558,13 +509,14 @@ fs_dirname_basename(const char *path,
     char **dir, char **base,
     int allow_relative)
 {
+	int saved_errno = errno;
 	char *buf = NULL;
 	char *slash;
 	size_t len;
-	int rval;
+	errno = 0;
 
 	if (if_err(path == NULL || dir == NULL || base == NULL, EFAULT))
-		return -1;
+		goto err;
 
 	slen(path, PATH_MAX, &len);
 	memcpy(smalloc(&buf, len + 1),
@@ -591,12 +543,14 @@ fs_dirname_basename(const char *path,
 		sdup(".", PATH_MAX, dir);
 		*base = buf;
 	} else {
-		errno = EINVAL;
 		free_and_set_null(&buf);
-		return -1;
+		goto err;
 	}
 
+	reset_caller_errno(0);
 	return 0;
+err:
+	return with_fallback_errno(EINVAL);
 }
 
 /* TODO: why does this abort, but others
@@ -608,7 +562,8 @@ open_file_on_eintr(const char *path,
     struct stat *st)
 {
 	int saved_errno = errno;
-	int rval;
+	int rval = 0;
+	errno = 0;
 
 	if (path == NULL)
 		err_exit(EINVAL, "open_file_on_eintr: null path");
@@ -626,8 +581,17 @@ open_file_on_eintr(const char *path,
 		err_exit(errno,
 		    "%s: open_file_on_eintr: could not close", path);
 
+	reset_caller_errno(rval);
 	*fd = rval;
 
+	/* we don't care about edge case behaviour here,
+	   even if the next operation sets errno on success,
+	   because the open() call is our main concern.
+	   however, we also must preserve the new errno,
+	   assuming it changed above under the same edge case */
+
+	saved_errno = errno;
+
 	if (st != NULL) {
 		if (fstat(*fd, st) < 0)
 			err_exit(errno, "%s: stat", path);
@@ -639,7 +603,7 @@ open_file_on_eintr(const char *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;
+	errno = saved_errno; /* see previous comment */
 }
 
 
@@ -657,20 +621,24 @@ openat_on_eintr(int dirfd, const char *path,
 		    RESOLVE_NO_MAGICLINKS
 	};
 	int saved_errno = errno;
-	long rval;
+	long rval = 0;
+	errno = 0;
 
 	if (if_err(dirfd < 0, EBADF) ||
 	    if_err(path == NULL, EFAULT))
-		return set_errno(saved_errno, EIO);
+		goto err;
 
 	errno = 0;
 	while (sys_retry(saved_errno,
 	    rval = syscall(SYS_openat2, dirfd, path, &how, sizeof(how))));
 
 	if (rval == -1) /* avoid long->int UB for -1 */
-		return -1;
+		goto err;
 
+	reset_caller_errno(rval);
 	return (int)rval;
+err:
+	return with_fallback_errno(EIO); /* -1 */
 }
 #else /* regular openat on non-linux e.g. openbsd */
 int
@@ -678,16 +646,17 @@ openat_on_eintr(int dirfd, const char *path,
     int flags, mode_t mode)
 {
 	int saved_errno = errno;
-	int rval;
+	int rval = 0;
+	errno = 0;
 
 	if (if_err(dirfd < 0, EBADF) ||
 	    if_err(path == NULL, EFAULT))
-		return set_errno(saved_errno, EIO);
+		return with_fallback_errno(EIO);
 
-	errno = 0;
 	while (fs_retry(saved_errno,
 	    rval = openat(dirfd, path, flags, mode)));
 
+	reset_caller_errno(rval);
 	return rval;
 }
 #endif
@@ -697,12 +666,13 @@ lseek_on_eintr(int fd, off_t off, int whence,
     int loop_eagain, int loop_eintr)
 {
 	int saved_errno = errno;
-	off_t rval;
-
+	off_t rval = 0;
 	errno = 0;
+
 	while (off_retry(saved_errno,
 	    rval = lseek(fd, off, whence)));
 
+	reset_caller_errno(rval);
 	return rval;
 }
 
@@ -711,16 +681,17 @@ mkdirat_on_eintr(int dirfd,
     const char *path, mode_t mode)
 {
 	int saved_errno = errno;
-	int rval;
+	int rval = 0;
+	errno = 0;
 
 	if (if_err(dirfd < 0, EBADF) ||
 	    if_err(path == NULL, EFAULT))
-		return set_errno(saved_errno, EIO);
+		return with_fallback_errno(EIO);
 
-	errno = 0;
 	while (fs_retry(saved_errno,
 	    rval = mkdirat(dirfd, path, mode)));
 
+	reset_caller_errno(rval);
 	return rval;
 }
 
@@ -729,17 +700,18 @@ read_on_eintr(int fd,
     void *buf, size_t count)
 {
 	int saved_errno = errno;
-	ssize_t rval;
+	ssize_t rval = 0;
+	errno = 0;
 
 	if (if_err(buf == NULL, EFAULT) ||
 	    if_err(fd < 0, EBADF) ||
 	    if_err(count == 0, EINVAL))
-		return set_errno(saved_errno, EIO);
+		return with_fallback_errno(EIO);
 
-	errno = 0;
 	while (rw_retry(saved_errno,
 	    rval = read(fd, buf, count)));
 
+	reset_caller_errno(rval);
 	return rval;
 }
 
@@ -749,18 +721,19 @@ pread_on_eintr(int fd,
     off_t off)
 {
 	int saved_errno = errno;
-	ssize_t rval;
+	ssize_t rval = 0;
+	errno = 0;
 
 	if (if_err(buf == NULL, EFAULT) ||
 	    if_err(fd < 0, EBADF) ||
 	    if_err(off < 0, EFAULT) ||
 	    if_err(count == 0, EINVAL))
-		return set_errno(saved_errno, EIO);
+		return with_fallback_errno(EIO);
 
-	errno = 0;
 	while (rw_retry(saved_errno,
 	    rval = pread(fd, buf, count, off)));
 
+	reset_caller_errno(rval);
 	return rval;
 }
 
@@ -769,17 +742,18 @@ write_on_eintr(int fd,
     void *buf, size_t count)
 {
 	int saved_errno = errno;
-	ssize_t rval;
+	ssize_t rval = 0;
+	errno = 0;
 
 	if (if_err(buf == NULL, EFAULT) ||
 	    if_err(fd < 0, EBADF) ||
 	    if_err(count == 0, EINVAL))
-		return set_errno(saved_errno, EIO);
+		return with_fallback_errno(EIO);
 
-	errno = 0;
 	while (rw_retry(saved_errno,
 	    rval = write(fd, buf, count)));
 
+	reset_caller_errno(rval);
 	return rval;
 }
 
@@ -789,18 +763,19 @@ pwrite_on_eintr(int fd,
     off_t off)
 {
 	int saved_errno = errno;
-	ssize_t rval;
+	ssize_t rval = 0;
+	errno = 0;
 
 	if (if_err(buf == NULL, EFAULT) ||
 	    if_err(fd < 0, EBADF) ||
 	    if_err(off < 0, EFAULT) ||
 	    if_err(count == 0, EINVAL))
-		return set_errno(saved_errno, EIO);
+		return with_fallback_errno(EIO);
 
-	errno = 0;
 	while (rw_retry(saved_errno,
 	    rval = pwrite(fd, buf, count, off)));
 
+	reset_caller_errno(rval);
 	return rval;
 }
 
@@ -808,15 +783,16 @@ int
 fsync_on_eintr(int fd)
 {
 	int saved_errno = errno;
-	int rval;
+	int rval = 0;
+	errno = 0;
 
 	if (if_err(fd < 0, EBADF))
-		return set_errno(saved_errno, EIO);
+		return with_fallback_errno(EIO);
 
-	errno = 0;
 	while (fs_retry(saved_errno,
 	    rval = fsync(fd)));
 
+	reset_caller_errno(rval);
 	return rval;
 }
 
@@ -824,7 +800,7 @@ void
 close_on_eintr(int *fd)
 {
 	int saved_errno = errno;
-	int rval;
+	int rval = 0;
 
 	if (fd == NULL)
 		err_exit(EINVAL, "close_on_eintr: null pointer");
@@ -839,6 +815,8 @@ close_on_eintr(int *fd)
 		err_exit(errno, "close_on_eintr: could not close");
 
 	*fd = -1;
+
+	reset_caller_errno(rval);
 }
 
 /* unified eintr looping.