libreboot-utils: much stricter close() handling

remove close_warn and close_no_err

make close_on_eintr a void, and abort
on error instead of returning -1.

a failed file closure is a world-ending
event. burn accordingly.

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

5 files changed, 39 insertions, 94 deletions

diff --git a/util/libreboot-utils/include/common.h b/util/libreboot-utils/include/common.h
index 9ec8a0e3..d41c4a77 100644
--- a/util/libreboot-utils/include/common.h
+++ b/util/libreboot-utils/include/common.h
@@ -539,14 +539,12 @@ int secure_file(int *fd,
     int check_seek,
     int do_lock,
     mode_t mode);
-int close_on_eintr(int fd);
+void close_on_eintr(int *fd);
 int fsync_on_eintr(int fd);
 int fs_rename_at(int olddirfd, const char *old,
              int newdirfd, const char *new);
 int fs_open(const char *path, int flags);
-void close_no_err(int *fd);
 void free_and_set_null(char **buf);
-int close_warn(int *fd, char *s);
 struct filesystem *rootfs(void);
 int fs_resolve_at(int dirfd, const char *path, int flags);
 int fs_next_component(const char **p,
diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c
index fbb4e53f..2b1adca8 100644
--- a/util/libreboot-utils/lib/file.c
+++ b/util/libreboot-utils/lib/file.c
@@ -146,11 +146,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);
 
@@ -163,7 +159,7 @@ err_fsync_dir:
 		errno = EIO;
 
 	free_and_set_null(&dirbuf);
-	close_no_err(&dirfd);
+	close_on_eintr(&dirfd);
 
 	return -1;
 }
@@ -589,66 +585,30 @@ free_and_set_null(char **buf)
 	*buf = NULL;
 }
 
-/* also returns error code */
-int
-close_warn(int *fd, char *s)
-{
-	int saved_errno = errno;
-
-	if (fd == NULL) {
-		if (s != NULL)
-			fprintf(stderr, "FAIL: %s: bad fd ptr\n", s);
-		return -1;
-	}
-
-	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;
-	}
-
-	*fd = -1;
-	errno = saved_errno;
-
-	return 0;
-}
-
-/* TODO: remove this, and just check
- * err on every close. */
 void
-close_no_err(int *fd)
+close_on_eintr(int *fd)
 {
+	int r;
 	int saved_errno = errno;
 
-	if (fd == NULL || *fd < 0)
-		return;
+	if (fd == NULL)
+		err_exit(EINVAL, "close_on_eintr: null pointer");
 
-	(void) close_on_eintr(*fd);
-	*fd = -1;
-
-	errno = saved_errno;
-}
-
-/* TODO: make fd a pointer insttead
-	and automatically reset -1 here */
-int
-close_on_eintr(int fd)
-{
-	int r;
-	int saved_errno = errno;
+	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
@@ -761,7 +721,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,11 +734,11 @@ 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;
@@ -847,7 +807,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 4fa6bf72..f69954c8 100644
--- a/util/libreboot-utils/lib/io.c
+++ b/util/libreboot-utils/lib/io.c
@@ -239,8 +239,8 @@ write_to_gbe_bin(void)
 
 	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;
 
@@ -440,15 +440,8 @@ gbe_mv(void)
 
 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;
-		}
+		close_on_eintr(&f->gbe_fd);
 		f->gbe_fd = -1;
 
 		if (fsync_dir(f->fname) < 0) {
@@ -457,13 +450,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
diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c
index f3a0087b..2726cb02 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
@@ -348,8 +348,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 +360,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,8 +374,8 @@ 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;
@@ -468,7 +468,7 @@ world_writeable_and_sticky(
 
 sticky_heaven:
 
-	close_no_err(&dirfd);
+	close_on_eintr(&dirfd);
 	errno = saved_errno;
 
 	return 1;
@@ -478,7 +478,7 @@ sticky_hell:
 	if (errno == saved_errno)
 		errno = EPERM;
 
-	close_no_err(&dirfd);
+	close_on_eintr(&dirfd);
 
 	return 0;
 }
@@ -606,7 +606,7 @@ mkhtemp(int *fd,
 	errno = EEXIST;
 
 err:
-	close_no_err(fd);
+	close_on_eintr(fd);
 
 success:
 	free_and_set_null(&fname_copy);
@@ -732,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);
@@ -827,7 +827,7 @@ err:
 	if (linked)
 		(void) unlinkat(dirfd, fname_copy, 0);
 
-	close_no_err(&tmpfd);
+	close_on_eintr(&tmpfd);
 	return -1;
 }
 #endif
diff --git a/util/libreboot-utils/lib/rand.c b/util/libreboot-utils/lib/rand.c
index 3ca19d0c..99da713c 100644
--- a/util/libreboot-utils/lib/rand.c
+++ b/util/libreboot-utils/lib/rand.c
@@ -169,7 +169,7 @@ retry_rand:
 
 #if defined(USE_URANDOM) && \
     ((USE_URANDOM) > 0)
-	close_no_err(&fd);
+	close_on_eintr(&fd);
 #endif
 	goto out;
 #endif
@@ -179,7 +179,7 @@ out:
 err:
 #if defined(USE_URANDOM) && \
     ((USE_URANDOM) > 0)
-	close_no_err(&fd);
+	close_on_eintr(&fd);
 #endif
 	err_exit(ECANCELED,
 	    "Randomisation failure, possibly unsupported in your kernel");