fix uid/gid > 2^31

This MR revisits a partial fix from 2018. The problem stems from incorrect
handling of unsigned 32-bit uid_ts and gid_ts as signed when values are
large - i.e. when the high bit is set. In that case, pgrep and pkill fail to
identify processes by uid. (They succeed when finding the same processes by
username.) The primary fix for this is to impliment the "FIXME" comment in
proc/readproc.h, the implementation of which allows the removal of the (int)
casts from the partial fix from 2018.

The other fixed code in this MR consists of tests in strict_atol() that
detects and errors out on overflows.

References:
 Merge !146

diff --git a/NEWS b/NEWS
index 5f41025d8901430cb9f8c317dd91474cd733c86c..831346383077106cac6ca4df12890205c3667649 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ procps-ng-NEXT
   * library: add support for accessing autogroups
   * pkill: Check for lt- variants of program name          issue #192
   * pgrep: Add newline after regex error message           merge #91
+  * pgrep: Fix selection where uid/gid > 2^31              merge !146
   * ps: Add OOM and OOMADJ fields                          issue #198
   * ps: Add IO Accounting fields                           issue #184
   * ps: Add PSS and USS fields                             issue #112

diff --git a/pgrep.c b/pgrep.c
index 71e82650b3c35af9ce3adc17050ea137f6f31c11..c5cd6f1d064175d22aa8cbbc3a64dcf5d947f418 100644 (file)
--- a/pgrep.c
+++ b/pgrep.c
@@ -247,8 +247,12 @@ static int strict_atol (const char *restrict str, long *restrict value)
 
     for ( ; *str; ++str) {
         if (! isdigit (*str))
-            return (0);
+            return 0;
+        if (res >= LONG_MAX / 10)
+            return 0;
         res *= 10;
+        if (res >= LONG_MAX - (*str - '0'))
+            return 0;
         res += *str - '0';
     }
     *value = sign * res;
@@ -323,7 +327,7 @@ static int conv_uid (const char *restrict name, struct el *restrict e)
         xwarnx(_("invalid user name: %s"), name);
         return 0;
     }
-    e->num = (int) pwd->pw_uid;
+    e->num = pwd->pw_uid;
     return 1;
 }
 
@@ -340,7 +344,7 @@ static int conv_gid (const char *restrict name, struct el *restrict e)
         xwarnx(_("invalid group name: %s"), name);
         return 0;
     }
-    e->num = (int) grp->gr_gid;
+    e->num = grp->gr_gid;
     return 1;
 }
 

diff --git a/proc/readproc.h b/proc/readproc.h
index 77db696735f50f05a61ee6b58f640e18ed9fbdba..55453c910c81e6c16dc8ca44ddc10870028d33bc 100644 (file)
--- a/proc/readproc.h
+++ b/proc/readproc.h
@@ -152,12 +152,13 @@ typedef struct proc_t {
         session,        // stat            session id
         nlwp,           // stat,status     number of threads, or 0 if no clue
         tgid,           // (special)       thread group ID, the POSIX PID (see also: tid)
-        tty,            // stat            full device number of controlling terminal
+        tty;            // stat            full device number of controlling terminal
         /* FIXME: int uids & gids should be uid_t or gid_t from pwd.h */
-        euid, egid,     // stat(),status   effective
-        ruid, rgid,     // status          real
-        suid, sgid,     // status          saved
-        fuid, fgid,     // status          fs (used for file access only)
+        uid_t euid; gid_t egid; // stat(),status effective
+        uid_t ruid; gid_t rgid; // status        real
+        uid_t suid; gid_t sgid; // status        saved
+        uid_t fuid; gid_t fgid; // status        fs (used for file access only)
+    int
         tpgid,          // stat            terminal process group id
         exit_signal,    // stat            might not be SIGCHLD
         processor;      // stat            current (or most recent?) CPU