emo/procps
1
0
Fork 0
Code Issues Pull Requests Packages Releases Activity

library: Better PID file checks

Browse Source 
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>
This commit is contained in:
Craig Small 2022-01-08 07:49:13 +11:00
parent 8b67402a25
commit 39d4d147d7
1 changed files with 16 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
proc/readproc.c
View File

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