From e500ba6d97c858931bd1409bd433c88bfffd4598 Mon Sep 17 00:00:00 2001 From: Todd Lewis Date: Mon, 25 Oct 2021 19:38:10 -0400 Subject: [PATCH] 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 --- NEWS | 1 + pgrep.c | 10 +++++++--- proc/readproc.h | 11 ++++++----- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index 5f41025d..83134638 100644 --- 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 71e82650..c5cd6f1d 100644 --- 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 77db6967..55453c91 100644 --- 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