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.

Submitted by:	rwatson  (original uiomove_frombuf -- bugs are mine :-)
Reported by:	Joost Pol <joost@pine.nl>  (integer underflows/overflows)

4 files changed, 26 insertions, 60 deletions

diff --git a/sys/fs/procfs/procfs_dbregs.c b/sys/fs/procfs/procfs_dbregs.c
index ae6e794..7ea4b91 100644
--- a/sys/fs/procfs/procfs_dbregs.c
+++ b/sys/fs/procfs/procfs_dbregs.c
@@ -65,31 +65,19 @@ procfs_doprocdbregs(PFS_FILL_ARGS)
 {
 	int error;
 	struct dbreg r;
-	char *kv;
-	int kl;
 
 	PROC_LOCK(p);
 	if (p_candebug(td, p) != 0) {
 		PROC_UNLOCK(p);
 		return (EPERM);
 	}
-	kl = sizeof(r);
-	kv = (char *) &r;
-
-	kv += uio->uio_offset;
-	kl -= uio->uio_offset;
-	if (kl > uio->uio_resid)
-		kl = uio->uio_resid;
 
 	_PHOLD(p);
-	if (kl < 0)
-		error = EINVAL;
-	else
-		/* XXXKSE: */
-		error = proc_read_dbregs(FIRST_THREAD_IN_PROC(p), &r);
+	/* XXXKSE: */
+	error = proc_read_dbregs(FIRST_THREAD_IN_PROC(p), &r);
 	if (error == 0) {
 		PROC_UNLOCK(p);
-		error = uiomove(kv, kl, uio);
+		error = uiomove_frombuf(&r, sizeof(r), uio);
 		PROC_LOCK(p);
 	}
 	if (error == 0 && uio->uio_rw == UIO_WRITE) {
diff --git a/sys/fs/procfs/procfs_fpregs.c b/sys/fs/procfs/procfs_fpregs.c
index b52f5ee..60150eb 100644
--- a/sys/fs/procfs/procfs_fpregs.c
+++ b/sys/fs/procfs/procfs_fpregs.c
@@ -59,31 +59,19 @@ procfs_doprocfpregs(PFS_FILL_ARGS)
 {
 	int error;
 	struct fpreg r;
-	char *kv;
-	int kl;
 
 	PROC_LOCK(p);
 	if (p_candebug(td, p)) {
 		PROC_UNLOCK(p);
 		return (EPERM);
 	}
-	kl = sizeof(r);
-	kv = (char *) &r;
-
-	kv += uio->uio_offset;
-	kl -= uio->uio_offset;
-	if (kl > uio->uio_resid)
-		kl = uio->uio_resid;
 
 	_PHOLD(p);
-	if (kl < 0)
-		error = EINVAL;
-	else
-		/* XXXKSE: */
-		error = proc_read_fpregs(FIRST_THREAD_IN_PROC(p), &r);
+	/* XXXKSE: */
+	error = proc_read_fpregs(FIRST_THREAD_IN_PROC(p), &r);
 	if (error == 0) {
 		PROC_UNLOCK(p);
-		error = uiomove(kv, kl, uio);
+		error = uiomove_frombuf(&r, sizeof(r), uio);
 		PROC_LOCK(p);
 	}
 	if (error == 0 && uio->uio_rw == UIO_WRITE) {
diff --git a/sys/fs/procfs/procfs_regs.c b/sys/fs/procfs/procfs_regs.c
index 1bdb58b..d3cad0f 100644
--- a/sys/fs/procfs/procfs_regs.c
+++ b/sys/fs/procfs/procfs_regs.c
@@ -59,31 +59,19 @@ procfs_doprocregs(PFS_FILL_ARGS)
 {
 	int error;
 	struct reg r;
-	char *kv;
-	int kl;
 
 	PROC_LOCK(p);
 	if (p_candebug(td, p)) {
 		PROC_UNLOCK(p);
 		return (EPERM);
 	}
-	kl = sizeof(r);
-	kv = (char *) &r;
-
-	kv += uio->uio_offset;
-	kl -= uio->uio_offset;
-	if (kl > uio->uio_resid)
-		kl = uio->uio_resid;
 
 	_PHOLD(p);
-	if (kl < 0)
-		error = EINVAL;
-	else
-		/* XXXKSE: */
-		error = proc_read_regs(FIRST_THREAD_IN_PROC(p), &r);
+	/* XXXKSE: */
+	error = proc_read_regs(FIRST_THREAD_IN_PROC(p), &r);
 	if (error == 0) {
 		PROC_UNLOCK(p);
-		error = uiomove(kv, kl, uio);
+		error = uiomove_frombuf(&r, sizeof(r), uio);
 		PROC_LOCK(p);
 	}
 	if (error == 0 && uio->uio_rw == UIO_WRITE) {
diff --git a/sys/fs/pseudofs/pseudofs_vnops.c b/sys/fs/pseudofs/pseudofs_vnops.c
index 0ac6dbc..28b1a65 100644
--- a/sys/fs/pseudofs/pseudofs_vnops.c
+++ b/sys/fs/pseudofs/pseudofs_vnops.c
@@ -34,6 +34,7 @@
 #include <sys/ctype.h>
 #include <sys/dirent.h>
 #include <sys/fcntl.h>
+#include <sys/limits.h>
 #include <sys/lock.h>
 #include <sys/mount.h>
 #include <sys/mutex.h>
@@ -479,8 +480,8 @@ pfs_read(struct vop_read_args *va)
 	struct uio *uio = va->a_uio;
 	struct proc *proc = NULL;
 	struct sbuf *sb = NULL;
-	char *ps;
-	int error, xlen;
+	int error;
+	unsigned int buflen, offset, resid;
 
 	PFS_TRACE((pn->pn_name));
 
@@ -515,7 +516,16 @@ pfs_read(struct vop_read_args *va)
 		PFS_RETURN (error);
 	}
 
-	sb = sbuf_new(sb, NULL, uio->uio_offset + uio->uio_resid, 0);
+	/* Beaucoup sanity checks so we don't ask for bogus allocation. */
+	if (uio->uio_offset < 0 || uio->uio_resid < 0 ||
+	    (offset = uio->uio_offset) != uio->uio_offset ||
+	    (resid = uio->uio_resid) != uio->uio_resid ||
+	    (buflen = offset + resid) < offset || buflen > INT_MAX) {
+		if (proc != NULL)
+			PRELE(proc);
+		PFS_RETURN (EINVAL);
+	}
+	sb = sbuf_new(sb, NULL, buflen, 0);
 	if (sb == NULL) {
 		if (proc != NULL)
 			PRELE(proc);
@@ -532,12 +542,8 @@ pfs_read(struct vop_read_args *va)
 		PFS_RETURN (error);
 	}
 
-	/* XXX we should possibly detect and handle overflows */
 	sbuf_finish(sb);
-	ps = sbuf_data(sb) + uio->uio_offset;
-	xlen = sbuf_len(sb) - uio->uio_offset;
-	xlen = imin(xlen, uio->uio_resid);
-	error = (xlen <= 0 ? 0 : uiomove(ps, xlen, uio));
+	error = uiomove_frombuf(sbuf_data(sb), sbuf_len(sb), uio);
 	sbuf_delete(sb);
 	PFS_RETURN (error);
 }
@@ -686,9 +692,9 @@ pfs_readlink(struct vop_readlink_args *va)
 	struct pfs_node *pn = pvd->pvd_pn;
 	struct uio *uio = va->a_uio;
 	struct proc *proc = NULL;
-	char buf[MAXPATHLEN], *ps;
+	char buf[MAXPATHLEN];
 	struct sbuf sb;
-	int error, xlen;
+	int error;
 
 	PFS_TRACE((pn->pn_name));
 
@@ -718,12 +724,8 @@ pfs_readlink(struct vop_readlink_args *va)
 		PFS_RETURN (error);
 	}
 
-	/* XXX we should detect and handle overflows */
 	sbuf_finish(&sb);
-	ps = sbuf_data(&sb) + uio->uio_offset;
-	xlen = sbuf_len(&sb) - uio->uio_offset;
-	xlen = imin(xlen, uio->uio_resid);
-	error = (xlen <= 0 ? 0 : uiomove(ps, xlen, uio));
+	error = uiomove_frombuf(sbuf_data(&sb), sbuf_len(&sb), uio);
 	sbuf_delete(&sb);
 	PFS_RETURN (error);
 }