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)

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)
+	/* XXXKSE: */
-		error = EINVAL;
+	error = proc_read_dbregs(FIRST_THREAD_IN_PROC(p), &r);
-	else
-		/* 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) {

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)
+	/* XXXKSE: */
-		error = EINVAL;
+	error = proc_read_fpregs(FIRST_THREAD_IN_PROC(p), &r);
-	else
-		/* 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) {

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)
+	/* XXXKSE: */
-		error = EINVAL;
+	error = proc_read_regs(FIRST_THREAD_IN_PROC(p), &r);
-	else
-		/* 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) {

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;
-	int error, xlen;
+	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;
+	error = uiomove_frombuf(sbuf_data(sb), sbuf_len(sb), uio);
-	xlen = sbuf_len(sb) - uio->uio_offset;
-	xlen = imin(xlen, uio->uio_resid);
-	error = (xlen <= 0 ? 0 : uiomove(ps, xlen, 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;
+	error = uiomove_frombuf(sbuf_data(&sb), sbuf_len(&sb), uio);
-	xlen = sbuf_len(&sb) - uio->uio_offset;
-	xlen = imin(xlen, uio->uio_resid);
-	error = (xlen <= 0 ? 0 : uiomove(ps, xlen, uio));
 	sbuf_delete(&sb);
 	PFS_RETURN (error);
 }

sys/kern/kern_subr.c

@@ -47,6 +47,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/systm.h>
 #include <sys/kernel.h>
 #include <sys/ktr.h>
+#include <sys/limits.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/proc.h>
@@ -198,6 +199,28 @@ uiomove(void *cp, int n, struct uio *uio)
 	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));
+}
+
 #ifdef ZERO_COPY_SOCKETS
 /*
  * Experimental support for zero-copy I/O

sys/sys/uio.h

@@ -87,6 +87,7 @@ struct vm_object;
 
 void	uio_yield(void);
 int	uiomove(void *, int, struct uio *);
+int	uiomove_frombuf(void *buf, int buflen, struct uio *uio);
 int	uiomoveco(void *, int, struct uio *, struct vm_object *, int);
 int	copyinfrom(const void *src, void *dst, size_t len, int seg);
 int	copyinstrfrom(const void *src, void *dst, size_t len,