library: Better PID file checks

This started off with fixing the compilier warning:

proc/readproc.c: In function ‘simple_nextpid’:
proc/readproc.c:1373:38: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 58 [-Wformat-truncation=]
 1373 |   snprintf(path, PROCPATHLEN, "/proc/%s", ent->d_name);
      |                                      ^~
proc/readproc.c:1373:3: note: ‘snprintf’ output between 7 and 262 bytes into a destination of size 64
 1373 |   snprintf(path, PROCPATHLEN, "/proc/%s", ent->d_name);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We know that ent->d_name will fit under 64 bytes because we check the
name starts with a digit. The first change was simple and changed the
printf to use tgid like the task function below it.

Is everything under /proc that starts with a digit a directory with a
PID only? Today, it is but there are no guarantees.

The entire function works ok if every non-pid directory doesn't start
with a number. We don't check for strtoul() having an issue nor
if the for loop just falls off the end. The moment the kernel guys
(or some module writer) think "/proc/12mykernelval" is a neat idea this
function is in trouble. We won't get buffer overflow as we are
using snprintf at least.

This change now:
 We check if strtoul() actually came across a number
 Process the pid directory as a conditional branch
 Treat falling off the for loop as a not-found

Signed-off-by: Craig Small <csmall@dropbear.xyz>

diff --git a/proc/readproc.c b/proc/readproc.c
index 442036409d7312f41d205ca19e1c324495781dc7..cc55cdfc33a79dfdfe735f7138d0a710a9b3557c 100644 (file)
--- a/proc/readproc.c
+++ b/proc/readproc.c
@@ -1361,17 +1361,22 @@ next_task:
 // This finds processes in /proc in the traditional way.
 // Return non-zero on success.
 static int simple_nextpid(PROCTAB *restrict const PT, proc_t *restrict const p) {
-  static __thread struct dirent *ent;   /* dirent handle */
-  char *restrict const path = PT->path;
-  for (;;) {
-    ent = readdir(PT->procfs);
-    if(!ent || !ent->d_name[0]) return 0;
-    if(*ent->d_name > '0' && *ent->d_name <= '9') break;
-  }
-  p->tgid = strtoul(ent->d_name, NULL, 10);
-  p->tid = p->tgid;
-  snprintf(path, PROCPATHLEN, "/proc/%s", ent->d_name);
-  return 1;
+    static __thread struct dirent *ent;   /* dirent handle */
+    char *restrict const path = PT->path;
+    for (;;) {
+        ent = readdir(PT->procfs);
+        if (!ent || !ent->d_name[0]) return 0;
+        if (*ent->d_name > '0' && *ent->d_name <= '9') {
+            errno = 0;
+            p->tgid = strtoul(ent->d_name, NULL, 10);
+            if (errno == 0) {
+                p->tid = p->tgid;
+                snprintf(path, PROCPATHLEN, "/proc/%d", p->tgid);
+                return 1;
+            }
+        }
+    }
+    return 0;
 }