Since we are able to change the device image resolution on the fly in the

meantime, the memory allocation for the read(2) method for video(4)
is not right anymore, and can cause a buffer overflow.

We fix this by queuering the maximum available image size for a device at
attach time.  If the image size should exceed our video(4) buffer after a
video format change (which shouldn't happen), uvideo(4) will gracefully
fail.

Also tested by kettenis@

3 files changed, 24 insertions, 9 deletions

diff --git a/sys/dev/usb/uvideo.c b/sys/dev/usb/uvideo.c
index 85eb843406b..eb2807ed68c 100644
--- a/sys/dev/usb/uvideo.c
+++ b/sys/dev/usb/uvideo.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvideo.c,v 1.63 2008/07/25 17:20:27 mglocker Exp $ */
+/*	$OpenBSD: uvideo.c,v 1.64 2008/07/26 11:42:43 mglocker Exp $ */
 
 /*
  * Copyright (c) 2008 Robert Nagy <robert@openbsd.org>
@@ -1119,8 +1119,6 @@ uvideo_vs_get_probe(struct uvideo_softc *sc, uint8_t *probe_data,
 	}
 	DPRINTF(1, "%s: GET probe request successfully\n", DEVNAME(sc));
 
-	sc->sc_video_buf_size = UGETDW(pc->dwMaxVideoFrameSize);
-
 	DPRINTF(1, "bmHint=0x%02x\n", UGETW(pc->bmHint));
 	DPRINTF(1, "bFormatIndex=0x%02x\n", pc->bFormatIndex);
 	DPRINTF(1, "bFrameIndex=0x%02x\n", pc->bFrameIndex);
@@ -1131,7 +1129,7 @@ uvideo_vs_get_probe(struct uvideo_softc *sc, uint8_t *probe_data,
 	DPRINTF(1, "wCompWindowSize=%d\n", UGETW(pc->wCompWindowSize));
 	DPRINTF(1, "wDelay=%d (ms)\n", UGETW(pc->wDelay));
 	DPRINTF(1, "dwMaxVideoFrameSize=%d (bytes)\n",
-	    sc->sc_video_buf_size);
+	    UGETDW(pc->dwMaxVideoFrameSize));
 	DPRINTF(1, "dwMaxPayloadTransferSize=%d (bytes)\n",
 	    UGETDW(pc->dwMaxPayloadTransferSize));
 
@@ -1172,7 +1170,7 @@ uvideo_vs_alloc_sample(struct uvideo_softc *sc)
 	fb->buf_size = UGETDW(sc->sc_desc_probe.dwMaxVideoFrameSize);
 
 	/* don't overflow the upper layer sample buffer */
-	if (sc->sc_video_buf_size < fb->buf_size) {
+	if (sc->sc_max_fbuf_size < fb->buf_size) {
 		printf("%s: sofware video buffer is too small!\n", DEVNAME(sc));
 		return (USBD_NOMEM);
 	}
@@ -2432,8 +2430,21 @@ int
 uvideo_get_bufsize(void *v)
 {
 	struct uvideo_softc *sc = v;
+	struct usb_video_probe_commit *pc;
+	uint8_t probe_data[34];
+	usbd_status error;
+
+	pc = (struct usb_video_probe_commit *)probe_data;
+
+	/* find the maximum frame size */
+	bzero(probe_data, sizeof(probe_data));
+	error = uvideo_vs_get_probe(sc, probe_data, GET_MAX);
+	if (error != USBD_NORMAL_COMPLETION)
+		return (EINVAL);
+
+	sc->sc_max_fbuf_size = UGETDW(pc->dwMaxVideoFrameSize);
 
-	return (sc->sc_video_buf_size);
+	return (sc->sc_max_fbuf_size);
 }
 
 void
diff --git a/sys/dev/usb/uvideo.h b/sys/dev/usb/uvideo.h
index 95bfe472f05..dcb55cc1c74 100644
--- a/sys/dev/usb/uvideo.h
+++ b/sys/dev/usb/uvideo.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvideo.h,v 1.24 2008/07/24 14:59:44 mglocker Exp $ */
+/*	$OpenBSD: uvideo.h,v 1.25 2008/07/26 11:42:43 mglocker Exp $ */
 
 /*
  * Copyright (c) 2007 Robert Nagy <robert@openbsd.org>
@@ -476,7 +476,7 @@ struct uvideo_softc {
 	int					 sc_enabled;
 	int					 sc_dying;
 	int					 sc_mode;
-	int					 sc_video_buf_size;
+	int					 sc_max_fbuf_size;
 	int					 sc_negotiated_flag;
 
 	u_int16_t				 uvc_version;
diff --git a/sys/dev/video.c b/sys/dev/video.c
index cfa33d97126..f2be15e1979 100644
--- a/sys/dev/video.c
+++ b/sys/dev/video.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: video.c,v 1.18 2008/07/23 22:10:21 mglocker Exp $	*/
+/*	$OpenBSD: video.c,v 1.19 2008/07/26 11:42:43 mglocker Exp $	*/
 /*
  * Copyright (c) 2008 Robert Nagy <robert@openbsd.org>
  * Copyright (c) 2008 Marcus Glocker <mglocker@openbsd.org>
@@ -77,6 +77,10 @@ videoattach(struct device *parent, struct device *self, void *aux)
 
 	if (sc->hw_if->get_bufsize)
 		video_buf_size = (sc->hw_if->get_bufsize)(sc->hw_hdl);
+	if (video_buf_size == EINVAL) {
+		printf("video: could not request frame buffer size\n");
+		return;
+	}
 
 	sc->sc_fbuffer = malloc(video_buf_size, M_DEVBUF, M_NOWAIT);
 	if (sc->sc_fbuffer == NULL) {