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@

diff --git a/sys/dev/usb/uvideo.c b/sys/dev/usb/uvideo.c
index 85eb843..eb2807e 100644 (file)
--- 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 95bfe47..dcb55cc 100644 (file)
--- 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 cfa33d9..f2be15e 100644 (file)
--- 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) {