From b8c8516a7f3390279d899a0ae5f3d8d584c07531 Mon Sep 17 00:00:00 2001 From: Eivind Eklund Date: Wed, 8 Nov 2000 21:53:05 +0000 Subject: [PATCH] More paranoia against overflows --- sys/fs/procfs/procfs_status.c | 63 ++++++++++++++++++++++--------- sys/miscfs/procfs/procfs_status.c | 63 ++++++++++++++++++++++--------- 2 files changed, 90 insertions(+), 36 deletions(-) diff --git a/sys/fs/procfs/procfs_status.c b/sys/fs/procfs/procfs_status.c index 53df2c84d4e4..b06a0af1aa5e 100644 --- a/sys/fs/procfs/procfs_status.c +++ b/sys/fs/procfs/procfs_status.c @@ -55,6 +55,7 @@ #include #include +#define DOCHECK() do { if (ps >= psbuf+sizeof(psbuf)) goto bailout; } while (0) int procfs_dostatus(curp, p, pfs, uio) struct proc *curp; @@ -71,7 +72,7 @@ procfs_dostatus(curp, p, pfs, uio) int i; int xlen; int error; - char psbuf[256+MAXHOSTNAMELEN]; /* XXX - conservative */ + char psbuf[256]; /* XXX - conservative */ if (uio->uio_rw != UIO_READ) return (EOPNOTSUPP); @@ -85,62 +86,85 @@ procfs_dostatus(curp, p, pfs, uio) /* comm pid ppid pgid sid maj,min ctty,sldr start ut st wmsg euid ruid rgid,egid,groups[1 .. NGROUPS] */ + KASSERT(sizeof(psbuf) > MAXCOMLEN, + ("Too short buffer for new MAXCOMLEN")); + ps = psbuf; bcopy(p->p_comm, ps, MAXCOMLEN); ps[MAXCOMLEN] = '\0'; ps += strlen(ps); - ps += sprintf(ps, " %d %d %d %d ", pid, ppid, pgid, sid); - + DOCHECK(); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + " %d %d %d %d ", pid, ppid, pgid, sid); + DOCHECK(); if ((p->p_flag&P_CONTROLT) && (tp = sess->s_ttyp)) - ps += sprintf(ps, "%d,%d ", major(tp->t_dev), minor(tp->t_dev)); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + "%d,%d ", major(tp->t_dev), minor(tp->t_dev)); else - ps += sprintf(ps, "%d,%d ", -1, -1); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + "%d,%d ", -1, -1); + DOCHECK(); sep = ""; if (sess->s_ttyvp) { - ps += sprintf(ps, "%sctty", sep); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "%sctty", sep); sep = ","; + DOCHECK(); } if (SESS_LEADER(p)) { - ps += sprintf(ps, "%ssldr", sep); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "%ssldr", sep); sep = ","; + DOCHECK(); + } + if (*sep != ',') { + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "noflags"); + DOCHECK(); } - if (*sep != ',') - ps += sprintf(ps, "noflags"); if (p->p_flag & P_INMEM) { struct timeval ut, st; calcru(p, &ut, &st, (struct timeval *) NULL); - ps += sprintf(ps, " %ld,%ld %ld,%ld %ld,%ld", + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + " %ld,%ld %ld,%ld %ld,%ld", p->p_stats->p_start.tv_sec, p->p_stats->p_start.tv_usec, ut.tv_sec, ut.tv_usec, st.tv_sec, st.tv_usec); } else - ps += sprintf(ps, " -1,-1 -1,-1 -1,-1"); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + " -1,-1 -1,-1 -1,-1"); + DOCHECK(); - ps += sprintf(ps, " %s", + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, " %s", (p->p_wchan && p->p_wmesg) ? p->p_wmesg : "nochan"); + DOCHECK(); cr = p->p_ucred; - ps += sprintf(ps, " %lu %lu %lu", + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, " %lu %lu %lu", (u_long)cr->cr_uid, (u_long)p->p_cred->p_ruid, (u_long)p->p_cred->p_rgid); + DOCHECK(); /* egid (p->p_cred->p_svgid) is equal to cr_ngroups[0] see also getegid(2) in /sys/kern/kern_prot.c */ - for (i = 0; i < cr->cr_ngroups; i++) - ps += sprintf(ps, ",%lu", (u_long)cr->cr_groups[i]); + for (i = 0; i < cr->cr_ngroups; i++) { + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + ",%lu", (u_long)cr->cr_groups[i]); + DOCHECK(); + } if (p->p_prison) - ps += sprintf(ps, " %s", p->p_prison->pr_host); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + " %s", p->p_prison->pr_host); else - ps += sprintf(ps, " -"); - ps += sprintf(ps, "\n"); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, " -"); + DOCHECK(); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "\n"); + DOCHECK(); xlen = ps - psbuf; xlen -= uio->uio_offset; @@ -152,6 +176,9 @@ procfs_dostatus(curp, p, pfs, uio) error = uiomove(ps, xlen, uio); return (error); + +bailout: + return (ENOMEM); } int diff --git a/sys/miscfs/procfs/procfs_status.c b/sys/miscfs/procfs/procfs_status.c index 53df2c84d4e4..b06a0af1aa5e 100644 --- a/sys/miscfs/procfs/procfs_status.c +++ b/sys/miscfs/procfs/procfs_status.c @@ -55,6 +55,7 @@ #include #include +#define DOCHECK() do { if (ps >= psbuf+sizeof(psbuf)) goto bailout; } while (0) int procfs_dostatus(curp, p, pfs, uio) struct proc *curp; @@ -71,7 +72,7 @@ procfs_dostatus(curp, p, pfs, uio) int i; int xlen; int error; - char psbuf[256+MAXHOSTNAMELEN]; /* XXX - conservative */ + char psbuf[256]; /* XXX - conservative */ if (uio->uio_rw != UIO_READ) return (EOPNOTSUPP); @@ -85,62 +86,85 @@ procfs_dostatus(curp, p, pfs, uio) /* comm pid ppid pgid sid maj,min ctty,sldr start ut st wmsg euid ruid rgid,egid,groups[1 .. NGROUPS] */ + KASSERT(sizeof(psbuf) > MAXCOMLEN, + ("Too short buffer for new MAXCOMLEN")); + ps = psbuf; bcopy(p->p_comm, ps, MAXCOMLEN); ps[MAXCOMLEN] = '\0'; ps += strlen(ps); - ps += sprintf(ps, " %d %d %d %d ", pid, ppid, pgid, sid); - + DOCHECK(); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + " %d %d %d %d ", pid, ppid, pgid, sid); + DOCHECK(); if ((p->p_flag&P_CONTROLT) && (tp = sess->s_ttyp)) - ps += sprintf(ps, "%d,%d ", major(tp->t_dev), minor(tp->t_dev)); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + "%d,%d ", major(tp->t_dev), minor(tp->t_dev)); else - ps += sprintf(ps, "%d,%d ", -1, -1); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + "%d,%d ", -1, -1); + DOCHECK(); sep = ""; if (sess->s_ttyvp) { - ps += sprintf(ps, "%sctty", sep); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "%sctty", sep); sep = ","; + DOCHECK(); } if (SESS_LEADER(p)) { - ps += sprintf(ps, "%ssldr", sep); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "%ssldr", sep); sep = ","; + DOCHECK(); + } + if (*sep != ',') { + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "noflags"); + DOCHECK(); } - if (*sep != ',') - ps += sprintf(ps, "noflags"); if (p->p_flag & P_INMEM) { struct timeval ut, st; calcru(p, &ut, &st, (struct timeval *) NULL); - ps += sprintf(ps, " %ld,%ld %ld,%ld %ld,%ld", + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + " %ld,%ld %ld,%ld %ld,%ld", p->p_stats->p_start.tv_sec, p->p_stats->p_start.tv_usec, ut.tv_sec, ut.tv_usec, st.tv_sec, st.tv_usec); } else - ps += sprintf(ps, " -1,-1 -1,-1 -1,-1"); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + " -1,-1 -1,-1 -1,-1"); + DOCHECK(); - ps += sprintf(ps, " %s", + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, " %s", (p->p_wchan && p->p_wmesg) ? p->p_wmesg : "nochan"); + DOCHECK(); cr = p->p_ucred; - ps += sprintf(ps, " %lu %lu %lu", + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, " %lu %lu %lu", (u_long)cr->cr_uid, (u_long)p->p_cred->p_ruid, (u_long)p->p_cred->p_rgid); + DOCHECK(); /* egid (p->p_cred->p_svgid) is equal to cr_ngroups[0] see also getegid(2) in /sys/kern/kern_prot.c */ - for (i = 0; i < cr->cr_ngroups; i++) - ps += sprintf(ps, ",%lu", (u_long)cr->cr_groups[i]); + for (i = 0; i < cr->cr_ngroups; i++) { + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + ",%lu", (u_long)cr->cr_groups[i]); + DOCHECK(); + } if (p->p_prison) - ps += sprintf(ps, " %s", p->p_prison->pr_host); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, + " %s", p->p_prison->pr_host); else - ps += sprintf(ps, " -"); - ps += sprintf(ps, "\n"); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, " -"); + DOCHECK(); + ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "\n"); + DOCHECK(); xlen = ps - psbuf; xlen -= uio->uio_offset; @@ -152,6 +176,9 @@ procfs_dostatus(curp, p, pfs, uio) error = uiomove(ps, xlen, uio); return (error); + +bailout: + return (ENOMEM); } int