6 files changed, 22 insertions, 44 deletions

diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c
index 5fdef7b3..3ca50889 100644
--- a/util/libreboot-utils/lib/file.c
+++ b/util/libreboot-utils/lib/file.c
@@ -153,7 +153,7 @@ fsync_dir(const char *path)
 		goto err_fsync_dir;
 	}
 
-	free_if_null(&dirbuf);
+	free_and_set_null(&dirbuf);
 
 	errno = saved_errno;
 	return 0;
@@ -163,7 +163,7 @@ err_fsync_dir:
 	if (errno == saved_errno)
 		errno = EIO;
 
-	free_if_null(&dirbuf);
+	free_and_set_null(&dirbuf);
 	close_no_err(&dirfd);
 
 	return -1;
@@ -581,7 +581,7 @@ try_err(int loop_err, int errval)
 }
 
 void
-free_if_null(char **buf)
+free_and_set_null(char **buf)
 {
 	if (buf == NULL || *buf == NULL)
 		return;
@@ -902,7 +902,7 @@ fs_dirname_basename(const char *path,
 		*base = buf;
 	} else {
 		errno = EINVAL;
-		free_if_null(&buf);
+		free_and_set_null(&buf);
 		return -1;
 	}
 
diff --git a/util/libreboot-utils/lib/io.c b/util/libreboot-utils/lib/io.c
index d05adbcc..1f2064a0 100644
--- a/util/libreboot-utils/lib/io.c
+++ b/util/libreboot-utils/lib/io.c
@@ -266,7 +266,7 @@ write_to_gbe_bin(void)
 
 			/* removed by rename
 			 */
-			free_if_null(&f->tname);
+			free_and_set_null(&f->tname);
 		}
 	}
 
diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c
index 0e0169e4..c913ce6c 100644
--- a/util/libreboot-utils/lib/mkhtemp.c
+++ b/util/libreboot-utils/lib/mkhtemp.c
@@ -184,7 +184,7 @@ err:
 	else
 		saved_errno = errno = EIO;
 
-	free_if_null(&dest);
+	free_and_set_null(&dest);
 
 	close_no_err(&dirfd);
 	close_no_err(fd);
@@ -619,7 +619,7 @@ err:
 	close_no_err(fd);
 
 success:
-	free_if_null(&fname_copy);
+	free_and_set_null(&fname_copy);
 
 	return (*fd >= 0) ? *fd : -1;
 }
diff --git a/util/libreboot-utils/lib/num.c b/util/libreboot-utils/lib/num.c
index 79d6b409..f53f0cee 100644
--- a/util/libreboot-utils/lib/num.c
+++ b/util/libreboot-utils/lib/num.c
@@ -33,44 +33,18 @@ hextonum(char ch_s)
 
 	ch = (unsigned char)ch_s;
 
-	if ((unsigned int)(ch - '0') <= 9) {
-
-		rval = ch - '0';
-		goto hextonum_success;
-	}
+	if ((unsigned int)(ch - '0') <= 9)
+		return ch - '0';
 
 	ch |= 0x20;
 
-	if ((unsigned int)(ch - 'a') <= 5) {
-
-		rval = ch - 'a' + 10;
-		goto hextonum_success;
-	}
-
-	if (ch == '?' || ch == 'x') {
-
-		rset(&rval, sizeof(rval));
-
-		goto hextonum_success;
-	}
-
-	goto err_hextonum;
-
-hextonum_success:
-
-	errno = saved_errno;
-	return (unsigned short)rval & 0xf;
-
-err_hextonum:
-
-	if (errno == saved_errno)
-		errno = EINVAL;
-	else
-		return 17; /* 17 indicates getrandom/urandom fail */
+	if ((unsigned int)(ch - 'a') <= 5)
+		return ch - 'a' + 10;
 
-	return 16; /* invalid character */
+	if (ch == '?' || ch == 'x')
+		return rsize(16); /* <-- with rejection sampling! */
 
-	/* caller just checks >15. */
+	return 16;
 }
 
 void
diff --git a/util/libreboot-utils/lib/rand.c b/util/libreboot-utils/lib/rand.c
index 3155eec3..10831e44 100644
--- a/util/libreboot-utils/lib/rand.c
+++ b/util/libreboot-utils/lib/rand.c
@@ -83,6 +83,10 @@ size_t
 rsize(size_t n)
 {
 	size_t rval = SIZE_MAX;
+	if (!n)
+		err_no_cleanup(0, EFAULT, "rsize: division by zero");
+
+	/* rejection sampling (clamp rand to eliminate modulo bias) */
 	for (; rval >= SIZE_MAX - (SIZE_MAX % n); rset(&rval, sizeof(rval)));
 
 	return rval % n;
@@ -115,7 +119,7 @@ mkrstr(size_t n) /* emulates spkmodem-decode */
 void *
 mkrbuf(size_t n)
 {
-	void *buf = "";
+	void *buf = NULL;
 
 	if (n == 0)
 		err_no_cleanup(0, EPERM, "mkrbuf: zero-byte request");
diff --git a/util/libreboot-utils/lib/state.c b/util/libreboot-utils/lib/state.c
index 42d060b7..41c851fb 100644
--- a/util/libreboot-utils/lib/state.c
+++ b/util/libreboot-utils/lib/state.c
@@ -139,7 +139,7 @@ xstart(int argc, char *argv[])
 	if (us.f.tmpbase == NULL)
 		err_no_cleanup(0, errno, "strdup tmpbase");
 
-	free_if_null(&tmpdir);
+	free_and_set_null(&tmpdir);
 
 	if (us.f.tname == NULL)
 		err_no_cleanup(0, errno, "x->f.tname null");
@@ -219,8 +219,8 @@ exit_cleanup(void)
 				close_err = 1;
 
 		close_no_err(&f->dirfd);
-		free_if_null(&f->base);
-		free_if_null(&f->tmpbase);
+		free_and_set_null(&f->base);
+		free_and_set_null(&f->tmpbase);
 	}
 
 	if (saved_errno)