Instead of relying on uvm_useracc(), get a false sense of security, and

do not check copyin() result, take care and properly handle copyin() failure.

This was not harmful, but a bit more correctness never harms.

6 files changed, 133 insertions, 97 deletions

diff --git a/sys/arch/macppc/pci/vgafb.c b/sys/arch/macppc/pci/vgafb.c
index c1a9f67f63f..3f0b981784b 100644
--- a/sys/arch/macppc/pci/vgafb.c
+++ b/sys/arch/macppc/pci/vgafb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vgafb.c,v 1.19 2002/09/15 09:01:59 deraadt Exp $	*/
+/*	$OpenBSD: vgafb.c,v 1.20 2002/11/09 22:51:46 miod Exp $	*/
 /*	$NetBSD: vga.c,v 1.3 1996/12/02 22:24:54 cgd Exp $	*/
 
 /*
@@ -491,18 +491,19 @@ vgafb_putcmap(vc, cm)
 {
 	u_int index = cm->index;
 	u_int count = cm->count;
-	int i;
+	u_int i;
+	int error;
 	u_int8_t *r, *g, *b;
 
 	if (index >= 256 || count > 256 - index)
 		return EINVAL;
-	if (!uvm_useracc(cm->red, count, B_READ) ||
-	    !uvm_useracc(cm->green, count, B_READ) ||
-	    !uvm_useracc(cm->blue, count, B_READ))
-		return EFAULT;
-	copyin(cm->red,   &(vc->vc_cmap_red[index]),   count);
-	copyin(cm->green, &(vc->vc_cmap_green[index]), count);
-	copyin(cm->blue,  &(vc->vc_cmap_blue[index]),  count);
+
+	if ((error = copyin(cm->red, &vc->vc_cmap_red[index], count)) != 0)
+		return (error);
+	if ((error = copyin(cm->green, &vc->vc_cmap_green[index], count)) != 0)
+		return (error);
+	if ((error = copyin(cm->blue, &vc->vc_cmap_blue[index], count)) != 0)
+		return (error);
 
 	r = &(vc->vc_cmap_red[index]);
 	g = &(vc->vc_cmap_green[index]);
diff --git a/sys/arch/sparc64/dev/vgafb.c b/sys/arch/sparc64/dev/vgafb.c
index bb1627b7717..783c02c7b83 100644
--- a/sys/arch/sparc64/dev/vgafb.c
+++ b/sys/arch/sparc64/dev/vgafb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vgafb.c,v 1.28 2002/09/15 14:29:29 miod Exp $	*/
+/*	$OpenBSD: vgafb.c,v 1.29 2002/11/09 22:51:48 miod Exp $	*/
 
 /*
  * Copyright (c) 2001 Jason L. Wright (jason@thought.net)
@@ -357,25 +357,27 @@ vgafb_putcmap(sc, cm)
 {
 	u_int index = cm->index;
 	u_int count = cm->count;
-	int i;
+	u_int i;
+	int error;
 	u_char *r, *g, *b;
 
 	if (index >= 256 || count > 256 - index)
 		return (EINVAL);
-	if (!uvm_useracc(cm->red, cm->count, B_READ) ||
-	    !uvm_useracc(cm->green, cm->count, B_READ) ||
-	    !uvm_useracc(cm->blue, cm->count, B_READ))
-		return (EFAULT);
-	copyin(cm->red, &sc->sc_cmap_red[index], count);
-	copyin(cm->green, &sc->sc_cmap_green[index], count);
-	copyin(cm->blue, &sc->sc_cmap_blue[index], count);
+
+	if ((error = copyin(cm->red, &sc->sc_cmap_red[index], count)) != 0)
+		return (error);
+	if ((error = copyin(cm->green, &sc->sc_cmap_green[index], count)) != 0)
+		return (error);
+	if ((error = copyin(cm->blue, &sc->sc_cmap_blue[index], count)) != 0)
+		return (error);
 
 	r = &sc->sc_cmap_red[index];
 	g = &sc->sc_cmap_green[index];
 	b = &sc->sc_cmap_blue[index];
 
 	for (i = 0; i < count; i++) {
-		OF_call_method("color!", sc->sc_ofhandle, 4, 0, *r, *g, *b, index);
+		OF_call_method("color!", sc->sc_ofhandle, 4, 0, *r, *g, *b,
+		    index);
 		r++, g++, b++, index++;
 	}
 	return (0);
diff --git a/sys/dev/ic/bt463.c b/sys/dev/ic/bt463.c
index f9204245cad..8e19391688c 100644
--- a/sys/dev/ic/bt463.c
+++ b/sys/dev/ic/bt463.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: bt463.c,v 1.9 2002/08/02 16:13:07 millert Exp $ */
+/* $OpenBSD: bt463.c,v 1.10 2002/11/09 22:51:48 miod Exp $ */
 /* $NetBSD: bt463.c,v 1.2 2000/06/13 17:21:06 nathanw Exp $ */
 
 /*-
@@ -366,23 +366,28 @@ bt463_set_cmap(rc, cmapp)
 {
 	struct bt463data *data = (struct bt463data *)rc;
 	u_int count, index;
-	int s;
+	int s, error;
 
-	if (cmapp->index >= BT463_NCMAP_ENTRIES ||
-	    cmapp->count > BT463_NCMAP_ENTRIES - cmapp->index)
+	index = cmapp->index;
+	count = cmapp->count;
+
+	if (index >= BT463_NCMAP_ENTRIES || count > BT463_NCMAP_ENTRIES - index)
 		return (EINVAL);
-	if (!uvm_useracc(cmapp->red, cmapp->count, B_READ) ||
-	    !uvm_useracc(cmapp->green, cmapp->count, B_READ) ||
-	    !uvm_useracc(cmapp->blue, cmapp->count, B_READ))
-		return (EFAULT);
 
 	s = spltty();
 
-	index = cmapp->index;
-	count = cmapp->count;
-	copyin(cmapp->red, &data->cmap_r[index], count);
-	copyin(cmapp->green, &data->cmap_g[index], count);
-	copyin(cmapp->blue, &data->cmap_b[index], count);
+	if ((error = copyin(cmapp->red, &data->cmap_r[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
+	if ((error = copyin(cmapp->green, &data->cmap_g[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
+	if ((error = copyin(cmapp->blue, &data->cmap_b[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
 
 	data->changed |= DATA_CMAP_CHANGED;
 
@@ -401,13 +406,12 @@ bt463_get_cmap(rc, cmapp)
 	u_int count, index;
 	int error;
 
-	if (cmapp->index >= BT463_NCMAP_ENTRIES ||
-	    cmapp->count > BT463_NCMAP_ENTRIES - cmapp->index)
-		return (EINVAL);
-
 	count = cmapp->count;
 	index = cmapp->index;
 
+	if (index >= BT463_NCMAP_ENTRIES || count > BT463_NCMAP_ENTRIES - index)
+		return (EINVAL);
+
 	error = copyout(&data->cmap_r[index], cmapp->red, count);
 	if (error)
 		return (error);
@@ -423,17 +427,23 @@ bt463_check_curcmap(rc, cursorp)
 	struct ramdac_cookie *rc;
 	struct wsdisplay_cursor *cursorp;
 {
-	int count;
+	u_int index, count;
+	u_int8_t spare[2];
+	int error;
+
+	index = cursorp->cmap.index;
+	count = cursorp->cmap.count;
 
-	if ((u_int)cursorp->cmap.index > 2 ||
-	    ((u_int)cursorp->cmap.index +
-	     (u_int)cursorp->cmap.count) > 2)
+	if (index >= 2 || count > 2 - index)
 		return (EINVAL);
-	count = cursorp->cmap.count; 
-	if (!uvm_useracc(cursorp->cmap.red, count, B_READ) ||
-	    !uvm_useracc(cursorp->cmap.green, count, B_READ) ||
-	    !uvm_useracc(cursorp->cmap.blue, count, B_READ))
-		return (EFAULT);
+
+	if ((error = copyin(&cursorp->cmap.red, &spare, count)) != 0)
+		return (error);
+	if ((error = copyin(&cursorp->cmap.green, &spare, count)) != 0)
+		return (error);
+	if ((error = copyin(&cursorp->cmap.blue, &spare, count)) != 0)
+		return (error);
+
 	return (0);
 }
 
diff --git a/sys/dev/ic/bt485.c b/sys/dev/ic/bt485.c
index d27b56aa4e3..2707d512aa5 100644
--- a/sys/dev/ic/bt485.c
+++ b/sys/dev/ic/bt485.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: bt485.c,v 1.11 2002/08/02 16:13:07 millert Exp $ */
+/* $OpenBSD: bt485.c,v 1.12 2002/11/09 22:51:48 miod Exp $ */
 /* $NetBSD: bt485.c,v 1.2 2000/04/02 18:55:01 nathanw Exp $ */
 
 /*
@@ -267,7 +267,7 @@ bt485_set_cmap(rc, cmapp)
 {
 	struct bt485data *data = (struct bt485data *)rc;
 	u_int count, index;
-	int s;
+	int s, error;
 
 #ifdef DIAGNOSTIC
 	if (rc == NULL)
@@ -275,20 +275,26 @@ bt485_set_cmap(rc, cmapp)
 	if (cmapp == NULL)
 		panic("bt485_set_cmap: cmapp");
 #endif
-	if (cmapp->index >= 256 || cmapp->count > 256 - cmapp->index)
+	index = cmapp->index;
+	count = cmapp->count;
+
+	if (index >= 256 || count > 256 - index)
 		return (EINVAL);
-	if (!uvm_useracc(cmapp->red, cmapp->count, B_READ) ||
-	    !uvm_useracc(cmapp->green, cmapp->count, B_READ) ||
-	    !uvm_useracc(cmapp->blue, cmapp->count, B_READ))
-		return (EFAULT);
 
 	s = spltty();
 
-	index = cmapp->index;
-	count = cmapp->count;
-	copyin(cmapp->red, &data->cmap_r[index], count);
-	copyin(cmapp->green, &data->cmap_g[index], count);
-	copyin(cmapp->blue, &data->cmap_b[index], count);
+	if ((error = copyin(cmapp->red, &data->cmap_r[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
+	if ((error = copyin(cmapp->green, &data->cmap_g[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
+	if ((error = copyin(cmapp->blue, &data->cmap_b[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
 
 	data->changed |= DATA_CMAP_CHANGED;
 
@@ -332,7 +338,9 @@ bt485_set_cursor(rc, cursorp)
 	struct wsdisplay_cursor *cursorp;
 {
 	struct bt485data *data = (struct bt485data *)rc;
-	int count, index, v, s;
+	u_int count, index;
+	int error;
+	int v, s;
 
 	v = cursorp->which;
 
@@ -341,24 +349,15 @@ bt485_set_cursor(rc, cursorp)
 	 * before we do anything that we can't recover from.
 	 */
 	if (v & WSDISPLAY_CURSOR_DOCMAP) {
-		if ((u_int)cursorp->cmap.index > 2 ||
-		    ((u_int)cursorp->cmap.index +
-		     (u_int)cursorp->cmap.count) > 2)
-			return (EINVAL);
+		index = cursorp->cmap.index;
 		count = cursorp->cmap.count;
-		if (!uvm_useracc(cursorp->cmap.red, count, B_READ) ||
-		    !uvm_useracc(cursorp->cmap.green, count, B_READ) ||
-		    !uvm_useracc(cursorp->cmap.blue, count, B_READ))
-			return (EFAULT);
+		if (index >= 2 || count > 2 - index)
+			return (EINVAL);
 	}
 	if (v & WSDISPLAY_CURSOR_DOSHAPE) {
 		if ((u_int)cursorp->size.x > CURSOR_MAX_SIZE ||
 		    (u_int)cursorp->size.y > CURSOR_MAX_SIZE)
 			return (EINVAL);
-		count = (CURSOR_MAX_SIZE / NBBY) * data->cursize.y;
-		if (!uvm_useracc(cursorp->image, count, B_READ) ||
-		    !uvm_useracc(cursorp->mask, count, B_READ))
-			return (EFAULT);
 	}
 
 	if (v & (WSDISPLAY_CURSOR_DOPOS | WSDISPLAY_CURSOR_DOCUR)) {
@@ -377,11 +376,23 @@ bt485_set_cursor(rc, cursorp)
 		data->changed |= DATA_ENB_CHANGED;
 	}
 	if (v & WSDISPLAY_CURSOR_DOCMAP) {
-		count = cursorp->cmap.count;
 		index = cursorp->cmap.index;
-		copyin(cursorp->cmap.red, &data->curcmap_r[index], count);
-		copyin(cursorp->cmap.green, &data->curcmap_g[index], count);
-		copyin(cursorp->cmap.blue, &data->curcmap_b[index], count);
+		count = cursorp->cmap.count;
+		if ((error = copyin(cursorp->cmap.red,
+		    &data->curcmap_r[index], count)) != 0) {
+			splx(s);
+			return (error);
+		}
+		if ((error = copyin(cursorp->cmap.green,
+		    &data->curcmap_g[index], count)) != 0) {
+			splx(s);
+			return (error);
+		}
+		if ((error = copyin(cursorp->cmap.blue,
+		    &data->curcmap_b[index], count)) != 0) {
+			splx(s);
+			return (error);
+		}
 		data->changed |= DATA_CURCMAP_CHANGED;
 	}
 	if (v & WSDISPLAY_CURSOR_DOSHAPE) {
@@ -389,8 +400,16 @@ bt485_set_cursor(rc, cursorp)
 		count = (CURSOR_MAX_SIZE / NBBY) * data->cursize.y;
 		bzero(data->curimage, sizeof data->curimage);
 		bzero(data->curmask, sizeof data->curmask);
-		copyin(cursorp->image, data->curimage, count);	/* can't fail */
-		copyin(cursorp->mask, data->curmask, count);	/* can't fail */
+		if ((error = copyin(cursorp->image, data->curimage,
+		    count)) != 0) {
+			splx(s);
+			return (error);
+		}
+		if ((error = copyin(cursorp->mask, data->curmask,
+		    count)) != 0) {
+			splx(s);
+			return (error);
+		}
 		data->changed |= DATA_CURSHAPE_CHANGED;
 	}
 
diff --git a/sys/dev/ic/ibm561.c b/sys/dev/ic/ibm561.c
index c862d4b343b..4b27812b80c 100644
--- a/sys/dev/ic/ibm561.c
+++ b/sys/dev/ic/ibm561.c
@@ -1,5 +1,5 @@
 /* $NetBSD: ibm561.c,v 1.1 2001/12/12 07:46:48 elric Exp $ */
-/* $OpenBSD: ibm561.c,v 1.2 2002/08/02 16:13:07 millert Exp $ */
+/* $OpenBSD: ibm561.c,v 1.3 2002/11/09 22:51:48 miod Exp $ */
 
 /*-
  * Copyright (c) 2001 The NetBSD Foundation, Inc.
@@ -275,22 +275,29 @@ ibm561_set_cmap(rc, cmapp)
 {
 	struct ibm561data *data = (struct ibm561data *)rc;
 	u_int count, index;
+	int error;
 	int s;
 
-	if (cmapp->index >= IBM561_NCMAP_ENTRIES ||
-	    cmapp->count > IBM561_NCMAP_ENTRIES - cmapp->index)
+	index = cmapp->index;
+	count = cmapp->count;
+
+	if (index >= IBM561_NCMAP_ENTRIES ||
+	    count > IBM561_NCMAP_ENTRIES - index)
 		return (EINVAL);
-	if (!uvm_useracc(cmapp->red, cmapp->count, B_READ) ||
-	    !uvm_useracc(cmapp->green, cmapp->count, B_READ) ||
-	    !uvm_useracc(cmapp->blue, cmapp->count, B_READ))
-		return (EFAULT);
 
 	s = spltty();
-	index = cmapp->index;
-	count = cmapp->count;
-	copyin(cmapp->red, &data->cmap_r[index], count);
-	copyin(cmapp->green, &data->cmap_g[index], count);
-	copyin(cmapp->blue, &data->cmap_b[index], count);
+	if ((error = copyin(cmapp->red, &data->cmap_r[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
+	if ((error = copyin(cmapp->green, &data->cmap_g[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
+	if ((error = copyin(cmapp->blue, &data->cmap_b[index], count)) != 0) {
+		splx(s);
+		return (error);
+	}
 	data->changed |= CHANGED_CMAP;
 	data->ramdac_sched_update(data->cookie, ibm561_update);
 	splx(s);
diff --git a/sys/dev/pci/tga.c b/sys/dev/pci/tga.c
index 12f7d978568..ec72344eaf8 100644
--- a/sys/dev/pci/tga.c
+++ b/sys/dev/pci/tga.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tga.c,v 1.18 2002/10/12 01:09:44 krw Exp $ */
+/* $OpenBSD: tga.c,v 1.19 2002/11/09 22:51:48 miod Exp $ */
 /* $NetBSD: tga.c,v 1.40 2002/03/13 15:05:18 ad Exp $ */
 
 /*
@@ -895,10 +895,6 @@ tga_builtin_set_cursor(dc, cursorp)
 		if ((u_int)cursorp->size.x != 64 ||
 		    (u_int)cursorp->size.y > 64)
 			return (EINVAL);
-		/* The cursor is 2 bits deep, and there is no mask */
-		count = (cursorp->size.y * 64 * 2) / NBBY;
-		if (!uvm_useracc(cursorp->image, count, B_READ))
-			return (EFAULT);
 	}
 	if (v & WSDISPLAY_CURSOR_DOHOT)		/* not supported */
 		return EINVAL;
@@ -921,12 +917,13 @@ tga_builtin_set_cursor(dc, cursorp)
 		dcrf->ramdac_set_curcmap(dcrc, cursorp);
 	}
 	if (v & WSDISPLAY_CURSOR_DOSHAPE) {
-		count = ((64 * 2) / NBBY) * cursorp->size.y;
+		/* The cursor is 2 bits deep, and there is no mask */
+		count = (cursorp->size.y * 64 * 2) / NBBY;
 		TGAWREG(dc, TGA_REG_CCBR,
 		    (TGARREG(dc, TGA_REG_CCBR) & ~0xfc00) | (cursorp->size.y << 10));
-		copyin(cursorp->image, (char *)(dc->dc_vaddr +
-		    (TGARREG(dc, TGA_REG_CCBR) & 0x3ff)),
-		    count);				/* can't fail. */
+		if ((error = copyin(cursorp->image,(char *)(dc->dc_vaddr +
+		    (TGARREG(dc, TGA_REG_CCBR) & 0x3ff)), count)) != 0)
+			return (error);
 	}
 	return (0);
 }