From 25e80b0639ae05d01f3b1be4e6763f1103430b9f Mon Sep 17 00:00:00 2001 From: David Rhodus Date: Thu, 2 Oct 2003 19:21:06 +0000 Subject: [PATCH] Introduce a uiomove_frombuf helper routine that handles computing and validating the offset within a given memory buffer before handing the real work off to uiomove(9). Use uiomove_frombuf in procfs to correct several issues with integer arithmetic that could result in underflows/overflows. As a side-effect, the code is significantly simplified. Add additional sanity checks when computing a memory allocation size in pfs_read. Reported by: Joost Pol (integer underflows/overflows) Originated from: FreeBSD --- sys/kern/kern_subr.c | 25 ++++++++++++++++++++++++- sys/sys/uio.h | 3 ++- sys/vfs/procfs/procfs_dbregs.c | 10 +++------- sys/vfs/procfs/procfs_fpregs.c | 10 +++------- sys/vfs/procfs/procfs_regs.c | 7 ++----- sys/vfs/procfs/procfs_rlimit.c | 5 ++--- sys/vfs/procfs/procfs_status.c | 6 +++--- 7 files changed, 39 insertions(+), 27 deletions(-) diff --git a/sys/kern/kern_subr.c b/sys/kern/kern_subr.c index e38f2c3bc6..864549a380 100644 --- a/sys/kern/kern_subr.c +++ b/sys/kern/kern_subr.c @@ -37,7 +37,7 @@ * * @(#)kern_subr.c 8.3 (Berkeley) 1/21/94 * $FreeBSD: src/sys/kern/kern_subr.c,v 1.31.2.2 2002/04/21 08:09:37 bde Exp $ - * $DragonFly: src/sys/kern/kern_subr.c,v 1.10 2003/08/03 12:29:05 hmp Exp $ + * $DragonFly: src/sys/kern/kern_subr.c,v 1.11 2003/10/02 19:21:06 drhodus Exp $ */ #include "opt_ddb.h" @@ -50,6 +50,7 @@ #include #include #include +#include #include #include @@ -124,6 +125,28 @@ uiomove(cp, n, uio) curproc->p_flag = (curproc->p_flag & ~P_DEADLKTREAT) | save; return (error); } +/* + * Wrapper for uiomove() that validates the arguments against a known-good + * kernel buffer.  Currently, uiomove accepts a signed (n) argument, which + * is almost definitely a bad thing, so we catch that here as well.  We + * return a runtime failure, but it might be desirable to generate a runtime + * assertion failure instead. + */ +int +uiomove_frombuf(void *buf, int buflen, struct uio *uio) +{ + unsigned int offset, n; + + if (uio->uio_offset < 0 || uio->uio_resid < 0 || + (offset = uio->uio_offset) != uio->uio_offset) + return (EINVAL); + if (buflen <= 0 || offset >= buflen) + return (0); + if ((n = buflen - offset) > INT_MAX) + return (EINVAL); + return (uiomove((char *)buf + offset, n, uio)); +} + int uiomoveco(cp, n, uio, obj) diff --git a/sys/sys/uio.h b/sys/sys/uio.h index 2f08f039b8..d6dfe3834c 100644 --- a/sys/sys/uio.h +++ b/sys/sys/uio.h @@ -32,7 +32,7 @@ * * @(#)uio.h 8.5 (Berkeley) 2/22/94 * $FreeBSD: src/sys/sys/uio.h,v 1.11.2.1 2001/09/28 16:58:35 dillon Exp $ - * $DragonFly: src/sys/sys/uio.h,v 1.4 2003/08/20 07:31:22 rob Exp $ + * $DragonFly: src/sys/sys/uio.h,v 1.5 2003/10/02 19:21:06 drhodus Exp $ */ #ifndef _SYS_UIO_H_ @@ -83,6 +83,7 @@ struct vm_object; void uio_yield (void); int uiomove (caddr_t, int, struct uio *); +int uiomove_frombuf (void *buf, int buflen, struct uio *uio); int uiomoveco (caddr_t, int, struct uio *, struct vm_object *); int uioread (int, struct uio *, struct vm_object *, int *); diff --git a/sys/vfs/procfs/procfs_dbregs.c b/sys/vfs/procfs/procfs_dbregs.c index 92fceb659a..eb9e25a8cf 100644 --- a/sys/vfs/procfs/procfs_dbregs.c +++ b/sys/vfs/procfs/procfs_dbregs.c @@ -41,7 +41,7 @@ * SUCH DAMAGE. * * $FreeBSD: src/sys/miscfs/procfs/procfs_dbregs.c,v 1.4.2.3 2002/01/22 17:22:59 nectar Exp $ - * $DragonFly: src/sys/vfs/procfs/procfs_dbregs.c,v 1.4 2003/08/07 21:17:43 dillon Exp $ + * $DragonFly: src/sys/vfs/procfs/procfs_dbregs.c,v 1.5 2003/10/02 19:21:06 drhodus Exp $ */ #include @@ -77,13 +77,9 @@ procfs_dodbregs(curp, p, pfs, uio) kl = uio->uio_resid; PHOLD(p); - - if (kl < 0) - error = EINVAL; - else - error = procfs_read_dbregs(p, &r); + error = procfs_read_dbregs(p, &r); if (error == 0) - error = uiomove(kv, kl, uio); + error = uiomove_frombuf(&r, sizeof(r), uio); if (error == 0 && uio->uio_rw == UIO_WRITE) { if (p->p_stat != SSTOP) error = EBUSY; diff --git a/sys/vfs/procfs/procfs_fpregs.c b/sys/vfs/procfs/procfs_fpregs.c index 7254191a4f..2b00c57da2 100644 --- a/sys/vfs/procfs/procfs_fpregs.c +++ b/sys/vfs/procfs/procfs_fpregs.c @@ -38,7 +38,7 @@ * * From: * $FreeBSD: src/sys/miscfs/procfs/procfs_fpregs.c,v 1.11.2.3 2002/01/22 17:22:59 nectar Exp $ - * $DragonFly: src/sys/vfs/procfs/procfs_fpregs.c,v 1.4 2003/08/07 21:17:43 dillon Exp $ + * $DragonFly: src/sys/vfs/procfs/procfs_fpregs.c,v 1.5 2003/10/02 19:21:06 drhodus Exp $ */ #include @@ -74,13 +74,9 @@ procfs_dofpregs(curp, p, pfs, uio) kl = uio->uio_resid; PHOLD(p); - - if (kl < 0) - error = EINVAL; - else - error = procfs_read_fpregs(p, &r); + error = procfs_read_fpregs(p, &r); if (error == 0) - error = uiomove(kv, kl, uio); + error = uiomove_frombuf(&r, sizeof(r), uio); if (error == 0 && uio->uio_rw == UIO_WRITE) { if (p->p_stat != SSTOP) error = EBUSY; diff --git a/sys/vfs/procfs/procfs_regs.c b/sys/vfs/procfs/procfs_regs.c index 66d3f3f52a..7c5381b7d6 100644 --- a/sys/vfs/procfs/procfs_regs.c +++ b/sys/vfs/procfs/procfs_regs.c @@ -38,7 +38,7 @@ * * From: * $FreeBSD: src/sys/miscfs/procfs/procfs_regs.c,v 1.10.2.3 2002/01/22 17:22:59 nectar Exp $ - * $DragonFly: src/sys/vfs/procfs/procfs_regs.c,v 1.4 2003/08/07 21:17:43 dillon Exp $ + * $DragonFly: src/sys/vfs/procfs/procfs_regs.c,v 1.5 2003/10/02 19:21:06 drhodus Exp $ */ #include @@ -76,12 +76,9 @@ procfs_doregs(curp, p, pfs, uio) PHOLD(p); - if (kl < 0) - error = EINVAL; - else error = procfs_read_regs(p, &r); if (error == 0) - error = uiomove(kv, kl, uio); + error = uiomove(&r, sizeof(r), uio); if (error == 0 && uio->uio_rw == UIO_WRITE) { if (p->p_stat != SSTOP) error = EBUSY; diff --git a/sys/vfs/procfs/procfs_rlimit.c b/sys/vfs/procfs/procfs_rlimit.c index 7ba04e7c1e..15d20bffa0 100644 --- a/sys/vfs/procfs/procfs_rlimit.c +++ b/sys/vfs/procfs/procfs_rlimit.c @@ -37,7 +37,7 @@ * @(#)procfs_status.c 8.4 (Berkeley) 6/15/94 * * $FreeBSD: src/sys/miscfs/procfs/procfs_rlimit.c,v 1.5 1999/12/08 08:59:37 phk Exp $ - * $DragonFly: src/sys/vfs/procfs/procfs_rlimit.c,v 1.3 2003/08/07 21:17:43 dillon Exp $ + * $DragonFly: src/sys/vfs/procfs/procfs_rlimit.c,v 1.4 2003/10/02 19:21:06 drhodus Exp $ */ /* @@ -122,8 +122,7 @@ procfs_dorlimit(curp, p, pfs, uio) if (xlen <= 0) error = 0; else - error = uiomove(ps, xlen, uio); - + error = uiomove_frombuf(psbuf, ps - psbuf, uio); return (error); } diff --git a/sys/vfs/procfs/procfs_status.c b/sys/vfs/procfs/procfs_status.c index 36d3926349..0878ba49be 100644 --- a/sys/vfs/procfs/procfs_status.c +++ b/sys/vfs/procfs/procfs_status.c @@ -38,7 +38,7 @@ * * From: * $FreeBSD: src/sys/miscfs/procfs/procfs_status.c,v 1.20.2.4 2002/01/22 17:22:59 nectar Exp $ - * $DragonFly: src/sys/vfs/procfs/procfs_status.c,v 1.4 2003/08/07 21:17:43 dillon Exp $ + * $DragonFly: src/sys/vfs/procfs/procfs_status.c,v 1.5 2003/10/02 19:21:06 drhodus Exp $ */ #include @@ -174,7 +174,7 @@ procfs_dostatus(curp, p, pfs, uio) if (xlen <= 0) error = 0; else - error = uiomove(ps, xlen, uio); + error = uiomove_frombuf(psbuf, ps - psbuf, uio); return (error); @@ -253,7 +253,7 @@ procfs_docmdline(curp, p, pfs, uio) if (xlen <= 0) error = 0; else - error = uiomove(ps, xlen, uio); + error = uiomove_frombuf(bp, buflen, uio); if (buf) FREE(buf, M_TEMP); return (error); -- 2.41.0