From 0721b0fec2fd3e262c1f03fd562aa0d9b7e06fd8 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sun, 26 Jul 2026 13:27:30 +0200 Subject: [PATCH] util: one whole-file reader for all tools Three private ones had grown: fnread() in util.c, flen() behind pid_cmdline()/pid_cgroup() in cgutil.c, and conf_read_template() in conf.c. Two of them were also wrong in ways the others were not. fnread() formatted the path into a char[256] and stat()ed it before opening, so a longer path was silently truncated and then read from whichever file the truncation happened to name, and the size could change between the look and the read. flen() existed because neither of those approaches works on procfs at all, where stat() reports zero and the only way to learn the size is to read to EOF. Add fslurp() to util.[ch], which every tool already links. It opens first and sizes the fd it holds, treats st_size as a hint, and reads until EOF, so procfs and regular files take the same path. Paths are formatted by libite's vfopenf(), which allocates to fit. Callers that need the byte count, /proc/PID/cmdline embeds NUL, ask for it. fnread() keeps its signature and becomes a bounded copy out of the result, so its one caller is unaffected. Signed-off-by: Joachim Wiberg --- src/cgutil.c | 69 +++++++------------------------- src/conf.c | 25 ++---------- src/util.c | 110 +++++++++++++++++++++++++++++++++++++++++++-------- src/util.h | 3 ++ 4 files changed, 115 insertions(+), 92 deletions(-) diff --git a/src/cgutil.c b/src/cgutil.c index a1d70a0c..9abd6bb5 100644 --- a/src/cgutil.c +++ b/src/cgutil.c @@ -34,6 +34,7 @@ #include "cgutil.h" #include "initctl.h" #include "log.h" +#include "util.h" #define CDIM plain ? "" : "\e[2m" #define CRST plain ? "" : "\e[0m" @@ -54,42 +55,18 @@ int cgroup_avail(void) return fismnt(FINIT_CGPATH); } -static size_t flen(FILE *fp) -{ - size_t total = 0, sz; - char buf[512]; - - while ((sz = fread(buf, 1, sizeof(buf), fp)) > 0) - total += sz; - rewind(fp); - - return total; -} - char *pid_cmdline(int pid) { size_t i, len; char *buf; - FILE *fp; - fp = fopenf("r", "/proc/%d/cmdline", pid); - if (!fp) + buf = fslurp(&len, "/proc/%d/cmdline", pid); + if (!buf) return strdup(""); /* regular process */ - len = flen(fp); if (len == 0) { - fail: - fclose(fp); - return NULL; /* kernel thread */ - } - - buf = calloc(len + 1, 1); - if (!buf) - goto fail; - - if (fread(buf, 1, len, fp) < len) { free(buf); - goto fail; + return NULL; /* kernel thread */ } /* replace all NUL chars with space */ @@ -98,46 +75,30 @@ char *pid_cmdline(int pid) buf[i] = ' '; } - fclose(fp); return buf; } char *pid_cgroup(int pid) { - char *buf, *ptr = NULL; - size_t len; - FILE *fp; + char *buf, *ptr; - fp = fopenf("r", "/proc/%d/cgroup", pid); - if (!fp) + buf = fslurp(NULL, "/proc/%d/cgroup", pid); + if (!buf) return NULL; - len = flen(fp); - if (len == 0) { - fclose(fp); - return NULL; - } - len++; - - buf = calloc(1, len); - if (!buf) { - fclose(fp); - return NULL; - } - - if (fgets(buf, len, fp)) - ptr = chomp(buf); - fclose(fp); + ptr = strchr(buf, '\n'); /* first line only */ + if (ptr) + *ptr = 0; + chomp(buf); + ptr = strchr(buf, '/'); if (ptr) { - ptr = strchr(ptr, '/'); - if (ptr) { - memmove(buf, ptr, strlen(ptr) + 1); - return buf; - } + memmove(buf, ptr, strlen(ptr) + 1); + return buf; } free(buf); + return NULL; } diff --git a/src/conf.c b/src/conf.c index 70f349d6..9ed81922 100644 --- a/src/conf.c +++ b/src/conf.c @@ -1660,30 +1660,13 @@ static int is_new_format(char *file, char *buf) */ static char *conf_read_template(char *file, char *name) { - struct stat st; - char *buf = NULL; - size_t len; - FILE *fp; + char *buf; - fp = fopen(file, "r"); - if (!fp) + buf = fslurp(NULL, "%s", file); + if (!buf) return NULL; - /* fstat() the open fd, no window for the file to change under us */ - if (fstat(fileno(fp), &st) || st.st_size < 0) - goto out; - - buf = malloc((size_t)st.st_size + 1); - if (!buf) - goto out; - - len = fread(buf, 1, (size_t)st.st_size, fp); - buf[len] = 0; - buf = conf_instantiate(buf, name); -out: - fclose(fp); - - return buf; + return conf_instantiate(buf, name); } /* diff --git a/src/util.c b/src/util.c index 0dd7737d..4fe6a2db 100644 --- a/src/util.c +++ b/src/util.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #ifdef HAVE_TERMIOS_H # include @@ -335,34 +336,109 @@ int mksubsys(const char *dir, mode_t mode, char *user, char *group) return rc; } -int fnread(char *buf, size_t len, char *fmt, ...) +/* + * Read an open stream to EOF into a malloc()'ed, NUL terminated buffer. + * + * st_size is only a hint here, and zero on procfs, so the read loop + * runs until EOF rather than trusting it. Sizing from the already + * open fd also leaves no window for the file to change between the + * look and the read. + */ +static char *slurp(FILE *fp, size_t *lenp) { - char path[256]; - va_list ap; - FILE *fp; + size_t len = 0, size = BUFSIZ; + struct stat st; + char *buf; - va_start(ap, fmt); - vsnprintf(path, sizeof(path), fmt, ap); - va_end(ap); + if (!fstat(fileno(fp), &st) && st.st_size > 0 && (size_t)st.st_size > size) + size = (size_t)st.st_size; - if (!buf || !len) { - struct stat st; + buf = malloc(size + 1); + if (!buf) + return NULL; - if (stat(path, &st)) - return -1; + while (1) { + char *ptr; - return (ssize_t)st.st_size; + len += fread(&buf[len], 1, size - len, fp); + if (len < size) + break; /* EOF, or error caught below */ + + ptr = realloc(buf, size * 2 + 1); + if (!ptr) { + free(buf); + return NULL; + } + buf = ptr; + size *= 2; } - fp = fopen(path, "r"); - if (!fp) - return -1; + if (ferror(fp)) { + free(buf); + return NULL; + } - len = fread(buf, sizeof(char), len - 1, fp); buf[len] = 0; + if (lenp) + *lenp = len; + + return buf; +} + +/* + * Read a whole file into a malloc()'ed, NUL terminated buffer, which + * the caller frees. @lenp, when given, returns the number of bytes + * read; the content may itself contain NUL, e.g. /proc/PID/cmdline. + */ +char *vfslurp(size_t *lenp, const char *fmt, va_list ap) +{ + char *buf; + FILE *fp; + + fp = vfopenf("r", fmt, ap); + if (!fp) + return NULL; + + buf = slurp(fp, lenp); fclose(fp); - return (int)len; + return buf; +} + +char *fslurp(size_t *lenp, const char *fmt, ...) +{ + va_list ap; + char *buf; + + va_start(ap, fmt); + buf = vfslurp(lenp, fmt, ap); + va_end(ap); + + return buf; +} + +int fnread(char *buf, size_t len, char *fmt, ...) +{ + size_t dlen; + va_list ap; + char *data; + + va_start(ap, fmt); + data = vfslurp(&dlen, fmt, ap); + va_end(ap); + + if (!data) + return -1; + + if (buf && len) { + if (dlen > len - 1) + dlen = len - 1; + memcpy(buf, data, dlen); + buf[dlen] = 0; + } + free(data); + + return (int)dlen; } int fnwrite(char *value, char *fmt, ...) diff --git a/src/util.h b/src/util.h index 2af4c9b5..b0ae8614 100644 --- a/src/util.h +++ b/src/util.h @@ -29,6 +29,7 @@ #include #ifdef HAVE_TERMIOS_H #include +#include #include #include #include @@ -72,6 +73,8 @@ int getcgroup (char *buf, size_t len); int mksubsys (const char *dir, mode_t mode, char *user, char *group); +char *fslurp (size_t *lenp, const char *fmt, ...) __attribute__ ((format (printf, 2, 3))); +char *vfslurp (size_t *lenp, const char *fmt, va_list ap); int fnread (char *buf, size_t len, char *fmt, ...) __attribute__ ((format (printf, 3, 4))); int fnwrite (char *value, char *fmt, ...) __attribute__ ((format (printf, 2, 3))); int fngetint (char *path, int *val);