1 files changed, 78 insertions, 95 deletions

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;
 		}