diff options
author | Craig Small <csmall@dropbear.xyz> | 2022-01-08 07:49:13 +1100 |
---|---|---|
committer | Craig Small <csmall@dropbear.xyz> | 2022-01-08 07:49:13 +1100 |
commit | 39d4d147d7ab8c67697974ffba249bab793a7104 (patch) | |
tree | d5b2b6c10754cbb961895e9d5d68e78052f5c5b7 | |
parent | 8b67402a25ea356b821fc7a1309dc781e43948fe (diff) | |
download | procps-ng-39d4d147d7ab8c67697974ffba249bab793a7104.tar.gz |
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>
-rw-r--r-- | proc/readproc.c | 27 |
1 files changed, 16 insertions, 11 deletions
diff --git a/proc/readproc.c b/proc/readproc.c index 4420364..cc55cdf 100644 --- 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; } |