From da20b75beac750bf936c9c959f18bf4dce4bdf11 Mon Sep 17 00:00:00 2001 From: Leah Rowe Date: Mon, 30 Mar 2026 05:13:31 +0100 Subject: libreboot-utils: more flexible string usage i previously used error status and set return values indirectly. i still do that, but where possible, i also now return the real value. this is because these string functions can no longer return with error status; on error, they all abort. this forces the program maintainer to keep their code reliable, and removes the need to check the error status after using syscalls, because these libc wrappers mitigate that and make use of libc for you, including errors. this is part of a general effort to promote safe use of the C programming language, especially in libreboot! Signed-off-by: Leah Rowe --- util/libreboot-utils/include/common.h | 10 +++---- util/libreboot-utils/lib/command.c | 15 ++-------- util/libreboot-utils/lib/file.c | 14 ++++----- util/libreboot-utils/lib/mkhtemp.c | 35 +++++++++------------- util/libreboot-utils/lib/state.c | 13 +++++---- util/libreboot-utils/lib/string.c | 55 +++++++++++++++++------------------ util/libreboot-utils/mkhtemp.c | 11 +++---- 7 files changed, 65 insertions(+), 88 deletions(-) (limited to 'util') diff --git a/util/libreboot-utils/include/common.h b/util/libreboot-utils/include/common.h index 74620ca6..8276d6da 100644 --- a/util/libreboot-utils/include/common.h +++ b/util/libreboot-utils/include/common.h @@ -378,20 +378,20 @@ int xunveilx(const char *path, const char *permissions); int xpledgex(const char *promises, const char *execpromises); char *smalloc(char **buf, size_t size); void *vmalloc(void **buf, size_t size); -int slen(const char *scmp, size_t maxlen, +size_t slen(const char *scmp, size_t maxlen, size_t *rval); int vcmp(const void *s1, const void *s2, size_t n); int scmp(const char *a, const char *b, size_t maxlen, int *rval); int ccmp(const char *a, const char *b, size_t i, int *rval); -int sdup(const char *s, +char *sdup(const char *s, size_t n, char **dest); -int scatn(ssize_t sc, const char **sv, +char *scatn(ssize_t sc, const char **sv, size_t max, char **rval); -int scat(const char *s1, const char *s2, +char *scat(const char *s1, const char *s2, size_t n, char **dest); -int dcat(const char *s, size_t n, +void dcat(const char *s, size_t n, size_t off, char **dest1, char **dest2); /* numerical functions diff --git a/util/libreboot-utils/lib/command.c b/util/libreboot-utils/lib/command.c index a1e46e5f..3ee75628 100644 --- a/util/libreboot-utils/lib/command.c +++ b/util/libreboot-utils/lib/command.c @@ -57,10 +57,7 @@ sanitize_command_index(size_t c) err_exit(EINVAL, "cmd index %lu: empty str", (size_t)c); - if (slen(cmd->str, MAX_CMD_LEN +1, &rval) < 0) - err_exit(errno, "Could not get command length"); - - if (rval > MAX_CMD_LEN) { + if (slen(cmd->str, MAX_CMD_LEN +1, &rval) > MAX_CMD_LEN) { err_exit(EINVAL, "cmd index %lu: str too long: %s", (size_t)c, cmd->str); } @@ -109,10 +106,7 @@ set_cmd(int argc, char *argv[]) cmd = x->cmd[c].str; - if (scmp(argv[2], cmd, MAX_CMD_LEN, &rval) < 0) - err_exit(EINVAL, - "could not compare command strings"); - if (rval != 0) + if (scmp(argv[2], cmd, MAX_CMD_LEN, &rval)) continue; /* not the right command */ /* valid command found */ @@ -239,10 +233,7 @@ parse_mac_string(void) size_t rval; - if (slen(x->mac.str, 18, &rval) < 0) - err_exit(EINVAL, "Could not determine MAC length"); - - if (rval != 17) + if (slen(x->mac.str, 18, &rval) != 17) err_exit(EINVAL, "MAC address is the wrong length"); memset(mac->mac_buf, 0, sizeof(mac->mac_buf)); diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c index f35d9749..b9d31ad7 100644 --- a/util/libreboot-utils/lib/file.c +++ b/util/libreboot-utils/lib/file.c @@ -88,10 +88,7 @@ fsync_dir(const char *path) maxlen = 4096; #endif - if (if_err(path == NULL, EFAULT) || - if_err_sys(slen(path, maxlen, &pathlen) < 0) || - if_err(pathlen >= maxlen || pathlen < 0, EMSGSIZE) || - if_err(pathlen == 0, EINVAL)) + if (if_err(slen(path, maxlen, &pathlen) == 0, EINVAL)) goto err_fsync_dir; memcpy(smalloc(&dirbuf, pathlen + 1), @@ -841,11 +838,12 @@ fs_dirname_basename(const char *path, size_t maxlen = 4096; #endif - if (path == NULL || dir == NULL || base == NULL || - if_err_sys(slen(path, maxlen, &len) < 0)) + if (if_err(path == NULL || dir == NULL || base == NULL, EFAULT)) return -1; - memcpy(smalloc(&buf, len + 1), path, len + 1); + slen(path, maxlen, &len); + memcpy(smalloc(&buf, len + 1), + path, len + 1); /* strip trailing slashes */ while (len > 1 && buf[len - 1] == '/') @@ -865,7 +863,7 @@ fs_dirname_basename(const char *path, } } else if (allow_relative) { - *dir = strdup("."); + sdup(".", maxlen, dir); *base = buf; } else { errno = EINVAL; diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c index 2ef26d67..0560da47 100644 --- a/util/libreboot-utils/lib/mkhtemp.c +++ b/util/libreboot-utils/lib/mkhtemp.c @@ -120,8 +120,6 @@ new_tmp_common(int *fd, char **path, int type, if (tmpdir == NULL) goto err; - if (slen(tmpdir, maxlen, &dirlen) < 0) - goto err; if (*tmpdir == '\0') goto err; if (*tmpdir != '/') @@ -132,15 +130,12 @@ new_tmp_common(int *fd, char **path, int type, else templatestr = "tmp.XXXXXXXXXX"; - if (slen(templatestr, maxlen, &templatestr_len) < 0) - goto err; - /* may as well calculate in advance */ - destlen = dirlen + 1 + templatestr_len; + destlen = slen(tmpdir, maxlen, &dirlen) + 1 + + slen(templatestr, maxlen, &templatestr_len); /* full path: */ - if (scatn(3, (const char *[]) { tmpdir, "/", templatestr }, - maxlen, &dest) < 0) - goto err; + dest = scatn(3, (const char *[]) { tmpdir, "/", templatestr }, + maxlen, &dest); fname = dest + dirlen + 1; @@ -312,12 +307,10 @@ same_dir(const char *a, const char *b) /* optimisation: if both dirs are the same, we don't need - to check anything. sehr schnell: + to check anything. sehr schnell! */ - if (scmp(a, b, maxlen, &rval_scmp) < 0) - goto err_same_dir; /* bonus: scmp checks null for us */ - if (rval_scmp == 0) + if (!scmp(a, b, maxlen, &rval_scmp)) goto success_same_dir; fd_a = fs_open(a, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); @@ -550,19 +543,17 @@ mkhtemp(int *fd, if (if_err(fd == NULL || template == NULL || fname == NULL || st_dir_initial == NULL, EFAULT) || if_err(*fd >= 0, EEXIST) || - if_err(dirfd < 0, EBADF) - || - if_err_sys(slen(template, max_len, &template_len) < 0) || - if_err(template_len >= max_len, EMSGSIZE) - || - if_err_sys(slen(fname, max_len, &fname_len) < 0) || - if_err(fname == NULL, EINVAL) || - if_err(strrchr(fname, '/') != NULL, EINVAL)) + if_err(dirfd < 0, EBADF)) return -1; - for (end = template + template_len; /* count X */ + /* count X */ + for (end = template + slen(template, max_len, &template_len); end > template && *--end == 'X'; xc++); + fname_len = slen(fname, max_len, &fname_len); + if (if_err(strrchr(fname, '/') != NULL, EINVAL)) + return -1; + if (if_err(xc < 3 || xc > template_len, EINVAL) || if_err(fname_len > template_len, EOVERFLOW)) return -1; diff --git a/util/libreboot-utils/lib/state.c b/util/libreboot-utils/lib/state.c index bcf2ccbc..f0be5656 100644 --- a/util/libreboot-utils/lib/state.c +++ b/util/libreboot-utils/lib/state.c @@ -22,6 +22,12 @@ struct xstate * xstart(int argc, char *argv[]) { +#if defined(PATH_LEN) && \ + ((PATH_LEN) >= 256) + static size_t maxlen = PATH_LEN; +#else + static size_t maxlen = 4096; +#endif static int first_run = 1; static char *dir = NULL; static char *base = NULL; @@ -113,8 +119,7 @@ xstart(int argc, char *argv[]) err_exit(errno, "xstart: don't know CWD of %s", us.f.fname); - if ((us.f.base = strdup(base)) == NULL) - err_exit(errno, "strdup base"); + sdup(base, maxlen, &us.f.base); us.f.dirfd = fs_open(dir, O_RDONLY | O_DIRECTORY); @@ -128,9 +133,7 @@ xstart(int argc, char *argv[]) &tmpdir, &tmpbase_local, 0) < 0) err_exit(errno, "tmp basename"); - us.f.tmpbase = strdup(tmpbase_local); - if (us.f.tmpbase == NULL) - err_exit(errno, "strdup tmpbase"); + sdup(tmpbase_local, maxlen, &us.f.tmpbase); free_and_set_null(&tmpdir); diff --git a/util/libreboot-utils/lib/string.c b/util/libreboot-utils/lib/string.c index 7f336eb6..c083bd6d 100644 --- a/util/libreboot-utils/lib/string.c +++ b/util/libreboot-utils/lib/string.c @@ -179,7 +179,7 @@ err: return -1; out: errno = saved_errno; - return 0; + return *rval; } int ccmp(const char *a, const char *b, @@ -206,7 +206,7 @@ int ccmp(const char *a, const char *b, } /* strict word-based strlen */ -int +size_t slen(const char *s, size_t maxlen, size_t *rval) @@ -256,15 +256,15 @@ err: if (rval != NULL) *rval = 0; - err_exit(errno, "slen"); - return -1; + err_exit(errno, "slen"); /* abort */ + return 0; /* gcc15 is happy */ out: errno = saved_errno; - return 0; + return *rval; } /* strict word-based strdup */ -int +char * sdup(const char *s, size_t max, char **dest) { @@ -329,14 +329,14 @@ err: (void) set_errno(saved_errno, EFAULT); err_exit(errno, "sdup"); - return -1; + return NULL; out: errno = saved_errno; - return 0; + return *dest; } /* concatenate N number of strings */ -int +char * scatn(ssize_t sc, const char **sv, size_t max, char **rval) { @@ -356,14 +356,12 @@ scatn(ssize_t sc, const char **sv, if (if_err(sv[i] == NULL, EFAULT)) goto err; else if (i == 0) { - if (sdup(sv[0], max, &final) < 0) - goto err; + (void) sdup(sv[0], max, &final); continue; } rtmp = NULL; - if (scat(final, sv[i], max, &rtmp) < 0) - goto err; + scat(final, sv[i], max, &rtmp); free_and_set_null(&final); final = rtmp; @@ -372,7 +370,7 @@ scatn(ssize_t sc, const char **sv, errno = saved_errno; *rval = final; - return 0; + return *rval; err: free_and_set_null(&rcur); free_and_set_null(&rtmp); @@ -381,11 +379,11 @@ err: (void) set_errno(saved_errno, EFAULT); err_exit(errno, "scatn"); - return -1; + return NULL; } /* strict strcat */ -int +char * scat(const char *s1, const char *s2, size_t n, char **dest) { @@ -400,29 +398,31 @@ scat(const char *s1, const char *s2, slen(s1, n, &size1); slen(s2, n, &size2); - if (if_err(size1 > SIZE_MAX - size2 - 1, EOVERFLOW)) + if (if_err(size1 + > SIZE_MAX - size2 - 1, EOVERFLOW)) goto err; - memcpy(smalloc(&rval, size1 + size2 + 1), - s1, size1); + smalloc(&rval, size1 + size2 + 1); + + memcpy(rval, s1, size1); memcpy(rval + size1, s2, size2); *(rval + size1 + size2) = '\0'; *dest = rval; errno = saved_errno; - return 0; + return *dest; err: (void) set_errno(saved_errno, EINVAL); if (dest != NULL) *dest = NULL; err_exit(errno, "scat"); - return -1; + return NULL; } /* strict split/de-cat - off is where 2nd buffer will start from */ -int +void dcat(const char *s, size_t n, size_t off, char **dest1, char **dest2) @@ -435,9 +435,7 @@ dcat(const char *s, size_t n, if (if_err(dest1 == NULL || dest2 == NULL, EFAULT)) goto err; - slen(s, n, &size); - - if (if_err(size >= SIZE_MAX - 1, EOVERFLOW) || + if (if_err(slen(s, n, &size) >= SIZE_MAX - 1, EOVERFLOW) || if_err(off >= size, EOVERFLOW)) goto err; @@ -453,7 +451,7 @@ dcat(const char *s, size_t n, *dest2 = rval2; errno = saved_errno; - return 0; + return; err: *dest1 = *dest2 = NULL; @@ -463,8 +461,6 @@ err: (void) set_errno(saved_errno, EINVAL); err_exit(errno, "dcat"); - - return -1; } /* because no libc reimagination is complete @@ -603,8 +599,9 @@ lbsetprogname(char *argv0) static int set = 0; if (!set) { - if (argv0 == NULL || sdup(argv0, 4096, &progname) < 0) + if (argv0 == NULL) return "libreboot-utils"; + (void) sdup(argv0, 4096, &progname); set = 1; } diff --git a/util/libreboot-utils/mkhtemp.c b/util/libreboot-utils/mkhtemp.c index de86a2bf..f4c2b646 100644 --- a/util/libreboot-utils/mkhtemp.c +++ b/util/libreboot-utils/mkhtemp.c @@ -95,11 +95,7 @@ main(int argc, char *argv[]) /* custom template e.g. foo.XXXXXXXXXXXXXXXXXXXXX */ if (template != NULL) { - if (slen(template, maxlen, &tlen) < 0) - err_exit(EINVAL, - "invalid template"); - - for (p = template + tlen; + for (p = template + slen(template, maxlen, &tlen); p > template && *--p == 'X'; xc++); if (xc < 3) /* the gnu mktemp errs on less than 3 */ @@ -132,8 +128,9 @@ main(int argc, char *argv[]) err_exit(EFAULT, "bad string initialisation"); if (*s == '\0') err_exit(EFAULT, "empty string initialisation"); - if (slen(s, maxlen, &len) < 0) - err_exit(EFAULT, "unterminated string initialisiert"); + + slen(s, maxlen, &len); /* Nullterminierung prüfen */ + /* for good measure */ printf("%s\n", s); -- cgit v1.2.1