]> granicus.if.org Git - procps-ng/commitdiff
library: address major bug in the 'listed_nextpid' guy
authorJim Warner <james.warner@comcast.net>
Tue, 10 Aug 2021 05:00:00 +0000 (00:00 -0500)
committerCraig Small <csmall@dropbear.xyz>
Thu, 12 Aug 2021 11:34:11 +0000 (21:34 +1000)
Ever since 2003, the 'listed_nextpid' routine has been
misrepresenting its duties. Far from finding processes
in a list given to openproc, it just inserted the next
pid in that list into the passed proc_t as BOTH a tgid
and tid. There was no attempt to validate such values.

The net result is that tid & tgid were valid only with
a thread group leader. When called with a pid for some
sibling thread, the resulting tgid would be incorrect.

With this commit, our little function will now attempt
to validate both the tid and tgid. If this should fail
then the fallback position will be the same as what we
inherited. So we're no worse off & likely much better.

[ note that calling the function with a thread's pid ]
[ likely stems from 2011 when a 'readeither' routine ]
[ was added which dealt with both tasks and threads! ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
proc/readproc.c

index f06be5ebec8126ec536a5382f389665c564e87de..49a798c5fe3a8647653c0c9b757d520ca64e9472 100644 (file)
@@ -1405,16 +1405,29 @@ static int simple_nexttid(PROCTAB *restrict const PT, const proc_t *restrict con
 
 //////////////////////////////////////////////////////////////////////////////////
 // This "finds" processes in a list that was given to openproc().
-// Return non-zero on success. (tgid was handy)
-static int listed_nextpid(PROCTAB *restrict const PT, proc_t *restrict const p) {
-  char *restrict const path = PT->path;
-  pid_t tgid = *(PT->pids)++;
-  if(tgid){
-    snprintf(path, PROCPATHLEN, "/proc/%d", tgid);
-    p->tgid = tgid;
-    p->tid = tgid;  // they match for leaders
+// Return non-zero on success. (tgid is a real headache)
+static int listed_nextpid (PROCTAB *PT, proc_t *p) {
+  static struct utlbuf_s ub = { NULL, 0 };
+  pid_t pid = *(PT->pids)++;
+  char *path = PT->path;
+
+  if (pid) {
+    snprintf(path, PROCPATHLEN, "/proc/%d", pid);
+    p->tid = p->tgid = pid;        // this tgid may be a huge fib |
+
+    /* the 'status' directory is the only place where we find the |
+       task's real tgid. it's a bit expensive, but remember we're |
+       dealing with fewer processes, unlike the other 'next' guys |
+       (plus we need not parse the whole thing like status2proc)! | */
+
+    if (file2str(path, "status", &ub) != -1) {
+      char *str = strstr(ub.buf, "Tgid:");
+      if (str) {
+        p->tgid = atoi(str + 5);   // this tgid is the proper one |
+      }
+    }
   }
-  return tgid;
+  return pid;
 }