From: mglocker Date: Thu, 31 Jul 2008 15:26:25 +0000 (+0000) Subject: - Don't relay on bFormatIndex as an internal array index, since this X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=6e7dc0a31a0d7017826ac86307ec250d63552640;p=openbsd - Don't relay on bFormatIndex as an internal array index, since this field is unreliable and can start with any number. Use an own internal array index instead. - If the read buffer is too small, return a propper error to the calling functions. Just check the buffer size if we use the read(2) method since it doesn't affect mmap(2). Fixes kernel crashes seen with the M$ LifeCam NX-6000 and internal (laptop) Sonix chipsets. Tested by jcs@ (Sonix) and myself (NX-6000). OK deraadt@ --- diff --git a/sys/dev/usb/uvideo.c b/sys/dev/usb/uvideo.c index b4f8e23dd94..fba39e2b550 100644 --- a/sys/dev/usb/uvideo.c +++ b/sys/dev/usb/uvideo.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uvideo.c,v 1.65 2008/07/29 13:45:04 mglocker Exp $ */ +/* $OpenBSD: uvideo.c,v 1.66 2008/07/31 15:26:25 mglocker Exp $ */ /* * Copyright (c) 2008 Robert Nagy @@ -80,9 +80,9 @@ int uvideo_vs_parse_desc_format_uncompressed(struct uvideo_softc *, const usb_descriptor_t *); int uvideo_vs_parse_desc_frame(struct uvideo_softc *); int uvideo_vs_parse_desc_frame_mjpeg(struct uvideo_softc *, - const usb_descriptor_t *, int *); + const usb_descriptor_t *); int uvideo_vs_parse_desc_frame_uncompressed(struct uvideo_softc *, - const usb_descriptor_t *, int *); + const usb_descriptor_t *); int uvideo_vs_parse_desc_alt(struct uvideo_softc *, struct usb_attach_arg *uaa, int, int, int); int uvideo_vs_set_alt(struct uvideo_softc *, usbd_interface_handle, @@ -166,7 +166,7 @@ int uvideo_try_fmt(void *, struct v4l2_format *); */ caddr_t uvideo_mappage(void *, off_t, int); int uvideo_get_bufsize(void *); -void uvideo_start_read(void *); +int uvideo_start_read(void *); #define DEVNAME(_s) ((_s)->sc_dev.dv_xname) @@ -617,6 +617,8 @@ uvideo_vs_parse_desc_format(struct uvideo_softc *sc) desc = usb_desc_iter_next(&iter); } + sc->sc_fmtgrp_idx = 0; + return (0); } @@ -634,21 +636,23 @@ uvideo_vs_parse_desc_format_mjpeg(struct uvideo_softc *sc, return (-1); } - if (d->bFormatIndex == UVIDEO_MAX_FORMAT) { + if (sc->sc_fmtgrp_idx > UVIDEO_MAX_FORMAT) { printf("%s: too many MJPEG format descriptors found!\n", DEVNAME(sc)); return (-1); } - sc->sc_fmtgrp[d->bFormatIndex].format = (struct uvideo_format_desc *)d; - sc->sc_fmtgrp[d->bFormatIndex].format_dfidx = - sc->sc_fmtgrp[d->bFormatIndex].format->u.mjpeg.bDefaultFrameIndex; - sc->sc_fmtgrp[d->bFormatIndex].pixelformat = V4L2_PIX_FMT_MJPEG; + sc->sc_fmtgrp[sc->sc_fmtgrp_idx].format = + (struct uvideo_format_desc *)d; + sc->sc_fmtgrp[sc->sc_fmtgrp_idx].format_dfidx = + sc->sc_fmtgrp[sc->sc_fmtgrp_idx].format->u.mjpeg.bDefaultFrameIndex; + sc->sc_fmtgrp[sc->sc_fmtgrp_idx].pixelformat = V4L2_PIX_FMT_MJPEG; if (sc->sc_fmtgrp_cur == NULL) /* set MJPEG format */ - sc->sc_fmtgrp_cur = &sc->sc_fmtgrp[d->bFormatIndex]; + sc->sc_fmtgrp_cur = &sc->sc_fmtgrp[sc->sc_fmtgrp_idx]; + sc->sc_fmtgrp_idx++; sc->sc_fmtgrp_num++; return (0); @@ -669,16 +673,17 @@ uvideo_vs_parse_desc_format_uncompressed(struct uvideo_softc *sc, return (-1); } - if (d->bFormatIndex == UVIDEO_MAX_FORMAT) { + if (sc->sc_fmtgrp_idx > UVIDEO_MAX_FORMAT) { printf("%s: too many UNCOMPRESSED format descriptors found!\n", DEVNAME(sc)); return (-1); } - sc->sc_fmtgrp[d->bFormatIndex].format = (struct uvideo_format_desc *)d; - sc->sc_fmtgrp[d->bFormatIndex].format_dfidx = - sc->sc_fmtgrp[d->bFormatIndex].format->u.uc.bDefaultFrameIndex; - i = d->bFormatIndex; + sc->sc_fmtgrp[sc->sc_fmtgrp_idx].format = + (struct uvideo_format_desc *)d; + sc->sc_fmtgrp[sc->sc_fmtgrp_idx].format_dfidx = + sc->sc_fmtgrp[sc->sc_fmtgrp_idx].format->u.uc.bDefaultFrameIndex; + i = sc->sc_fmtgrp_idx; if (!strcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat, "YUY2")) { sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_YUYV; } else if (!strcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat, "NV12")) { @@ -689,8 +694,9 @@ uvideo_vs_parse_desc_format_uncompressed(struct uvideo_softc *sc, if (sc->sc_fmtgrp_cur == NULL) /* set UNCOMPRESSED format */ - sc->sc_fmtgrp_cur = &sc->sc_fmtgrp[d->bFormatIndex]; + sc->sc_fmtgrp_cur = &sc->sc_fmtgrp[sc->sc_fmtgrp_idx]; + sc->sc_fmtgrp_idx++; sc->sc_fmtgrp_num++; return (0); @@ -701,7 +707,6 @@ uvideo_vs_parse_desc_frame(struct uvideo_softc *sc) { usbd_desc_iter_t iter; const usb_descriptor_t *desc; - int fmtidx = 0; DPRINTF(1, "%s: %s\n", DEVNAME(sc), __func__); @@ -715,14 +720,14 @@ uvideo_vs_parse_desc_frame(struct uvideo_softc *sc) switch (desc->bDescriptorSubtype) { case UDESCSUB_VS_FRAME_MJPEG: - if (uvideo_vs_parse_desc_frame_mjpeg(sc, desc, &fmtidx)) + if (uvideo_vs_parse_desc_frame_mjpeg(sc, desc)) return (1); break; case UDESCSUB_VS_FRAME_UNCOMPRESSED: /* XXX do correct length calculation */ if (desc->bLength > 25) { if (uvideo_vs_parse_desc_frame_uncompressed(sc, - desc, &fmtidx)) + desc)) return (1); } break; @@ -736,58 +741,62 @@ uvideo_vs_parse_desc_frame(struct uvideo_softc *sc) int uvideo_vs_parse_desc_frame_mjpeg(struct uvideo_softc *sc, - const usb_descriptor_t *desc, int *fmtidx) + const usb_descriptor_t *desc) { struct usb_video_frame_mjpeg_desc *d; + int fmtidx; d = (struct usb_video_frame_mjpeg_desc *)(uint8_t *)desc; - if (d->bFrameIndex == 1) - ++*fmtidx; - if (d->bFrameIndex == UVIDEO_MAX_FRAME) { printf("%s: too many MJPEG frame descriptors found!\n", DEVNAME(sc)); return (1); } - sc->sc_fmtgrp[*fmtidx].frame[d->bFrameIndex] = d; + + fmtidx = sc->sc_fmtgrp_idx; + sc->sc_fmtgrp[fmtidx].frame[d->bFrameIndex] = d; /* * If bDefaultFrameIndex is not set by the device * use the first bFrameIndex available, otherwise * set it to the default one. */ - if (sc->sc_fmtgrp[*fmtidx].format->u.mjpeg.bDefaultFrameIndex == 0) { - sc->sc_fmtgrp[*fmtidx].frame_cur = - sc->sc_fmtgrp[*fmtidx].frame[1]; - } else if (sc->sc_fmtgrp[*fmtidx].format->u.mjpeg.bDefaultFrameIndex == + if (sc->sc_fmtgrp[fmtidx].format->u.mjpeg.bDefaultFrameIndex == 0) { + sc->sc_fmtgrp[fmtidx].frame_cur = + sc->sc_fmtgrp[fmtidx].frame[1]; + } else if (sc->sc_fmtgrp[fmtidx].format->u.mjpeg.bDefaultFrameIndex == d->bFrameIndex) { - sc->sc_fmtgrp[*fmtidx].frame_cur = - sc->sc_fmtgrp[*fmtidx].frame[d->bFrameIndex]; + sc->sc_fmtgrp[fmtidx].frame_cur = + sc->sc_fmtgrp[fmtidx].frame[d->bFrameIndex]; } - sc->sc_fmtgrp[*fmtidx].frame_num++; + sc->sc_fmtgrp[fmtidx].frame_num++; + + if (d->bFrameIndex == + sc->sc_fmtgrp[fmtidx].format->bNumFrameDescriptors) + sc->sc_fmtgrp_idx++; return (0); } int uvideo_vs_parse_desc_frame_uncompressed(struct uvideo_softc *sc, - const usb_descriptor_t *desc, int *fmtidx) + const usb_descriptor_t *desc) { struct usb_video_frame_uncompressed_desc *d; + int fmtidx; d = (struct usb_video_frame_uncompressed_desc *)(uint8_t *)desc; - if (d->bFrameIndex == 1) - ++*fmtidx; - if (d->bFrameIndex == UVIDEO_MAX_FRAME) { printf("%s: too many UNCOMPRESSED frame descriptors found!\n", DEVNAME(sc)); return (1); } - sc->sc_fmtgrp[*fmtidx].frame[d->bFrameIndex] = + + fmtidx = sc->sc_fmtgrp_idx; + sc->sc_fmtgrp[fmtidx].frame[d->bFrameIndex] = (struct usb_video_frame_mjpeg_desc *)d; /* @@ -795,16 +804,20 @@ uvideo_vs_parse_desc_frame_uncompressed(struct uvideo_softc *sc, * use the first bFrameIndex available, otherwise * set it to the default one. */ - if (sc->sc_fmtgrp[*fmtidx].format->u.uc.bDefaultFrameIndex == 0) { - sc->sc_fmtgrp[*fmtidx].frame_cur = - sc->sc_fmtgrp[*fmtidx].frame[1]; - } else if (sc->sc_fmtgrp[*fmtidx].format->u.uc.bDefaultFrameIndex == + if (sc->sc_fmtgrp[fmtidx].format->u.uc.bDefaultFrameIndex == 0) { + sc->sc_fmtgrp[fmtidx].frame_cur = + sc->sc_fmtgrp[fmtidx].frame[1]; + } else if (sc->sc_fmtgrp[fmtidx].format->u.uc.bDefaultFrameIndex == d->bFrameIndex) { - sc->sc_fmtgrp[*fmtidx].frame_cur = - sc->sc_fmtgrp[*fmtidx].frame[d->bFrameIndex]; + sc->sc_fmtgrp[fmtidx].frame_cur = + sc->sc_fmtgrp[fmtidx].frame[d->bFrameIndex]; } - sc->sc_fmtgrp[*fmtidx].frame_num++; + sc->sc_fmtgrp[fmtidx].frame_num++; + + if (d->bFrameIndex == + sc->sc_fmtgrp[fmtidx].format->bNumFrameDescriptors) + sc->sc_fmtgrp_idx++; return (0); } @@ -1170,7 +1183,8 @@ 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_max_fbuf_size < fb->buf_size) { + if (sc->sc_max_fbuf_size < fb->buf_size && + sc->sc_mmap_flag == 0) { printf("%s: sofware video buffer is too small!\n", DEVNAME(sc)); return (USBD_NOMEM); } @@ -2099,10 +2113,10 @@ uvideo_enum_fmt(void *v, struct v4l2_fmtdesc *fmtdesc) /* type not supported */ return (EINVAL); - idx = fmtdesc->index + 1; - if (idx > sc->sc_fmtgrp_num) + if (fmtdesc->index == sc->sc_fmtgrp_num) /* no more formats left */ return (EINVAL); + idx = fmtdesc->index; switch (sc->sc_fmtgrp[idx].format->bDescriptorSubtype) { case UDESCSUB_VS_FORMAT_MJPEG: @@ -2160,7 +2174,7 @@ uvideo_s_fmt(void *v, struct v4l2_format *fmt) DEVNAME(sc), __func__, fmt->fmt.pix.width, fmt->fmt.pix.height); /* search requested pixel format */ - for (found = 0, i = 1; i <= sc->sc_fmtgrp_num; i++) { + for (found = 0, i = 0; i < sc->sc_fmtgrp_num; i++) { if (fmt->fmt.pix.pixelformat == sc->sc_fmtgrp[i].pixelformat) { found = 1; break; @@ -2357,8 +2371,12 @@ int uvideo_streamon(void *v, int type) { struct uvideo_softc *sc = v; + usbd_status error; + + error = uvideo_vs_init(sc); + if (error != USBD_NORMAL_COMPLETION) + return (EINVAL); - uvideo_vs_init(sc); uvideo_vs_start(sc); return (0); @@ -2388,7 +2406,7 @@ uvideo_try_fmt(void *v, struct v4l2_format *fmt) DEVNAME(sc), __func__, fmt->fmt.pix.width, fmt->fmt.pix.height); /* search requested pixel format */ - for (found = 0, i = 1; i <= sc->sc_fmtgrp_num; i++) { + for (found = 0, i = 0; i < sc->sc_fmtgrp_num; i++) { if (fmt->fmt.pix.pixelformat == sc->sc_fmtgrp[i].pixelformat) { found = 1; break; @@ -2440,22 +2458,29 @@ uvideo_get_bufsize(void *v) /* 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) + if (error != USBD_NORMAL_COMPLETION) { return (EINVAL); + } sc->sc_max_fbuf_size = UGETDW(pc->dwMaxVideoFrameSize); return (sc->sc_max_fbuf_size); } -void +int uvideo_start_read(void *v) { struct uvideo_softc *sc = v; + usbd_status error; if (sc->sc_mmap_flag) sc->sc_mmap_flag = 0; - uvideo_vs_init(sc); + error = uvideo_vs_init(sc); + if (error != USBD_NORMAL_COMPLETION) + return (EINVAL); + uvideo_vs_start(sc); + + return (0); } diff --git a/sys/dev/usb/uvideo.h b/sys/dev/usb/uvideo.h index dcb55cc1c74..21b9aa5952e 100644 --- a/sys/dev/usb/uvideo.h +++ b/sys/dev/usb/uvideo.h @@ -1,4 +1,4 @@ -/* $OpenBSD: uvideo.h,v 1.25 2008/07/26 11:42:43 mglocker Exp $ */ +/* $OpenBSD: uvideo.h,v 1.26 2008/07/31 15:26:25 mglocker Exp $ */ /* * Copyright (c) 2007 Robert Nagy @@ -501,6 +501,7 @@ struct uvideo_softc { struct usb_video_input_header_desc_all sc_desc_vs_input_header; #define UVIDEO_MAX_FORMAT 8 + int sc_fmtgrp_idx; int sc_fmtgrp_num; struct uvideo_format_group *sc_fmtgrp_cur; struct uvideo_format_group sc_fmtgrp[UVIDEO_MAX_FORMAT]; diff --git a/sys/dev/video.c b/sys/dev/video.c index f2be15e1979..dd741a85ac6 100644 --- a/sys/dev/video.c +++ b/sys/dev/video.c @@ -1,4 +1,4 @@ -/* $OpenBSD: video.c,v 1.19 2008/07/26 11:42:43 mglocker Exp $ */ +/* $OpenBSD: video.c,v 1.20 2008/07/31 15:26:25 mglocker Exp $ */ /* * Copyright (c) 2008 Robert Nagy * Copyright (c) 2008 Marcus Glocker @@ -146,8 +146,10 @@ videoread(dev_t dev, struct uio *uio, int ioflag) /* start the stream */ if (sc->hw_if->start_read && !sc->sc_start_read) { + error = sc->hw_if->start_read(sc->hw_hdl); + if (error) + return (error); sc->sc_start_read = 1; - sc->hw_if->start_read(sc->hw_hdl); } DPRINTF(("resid=%d\n", uio->uio_resid)); diff --git a/sys/dev/video_if.h b/sys/dev/video_if.h index cc5c74486f8..69ff3174164 100644 --- a/sys/dev/video_if.h +++ b/sys/dev/video_if.h @@ -1,4 +1,4 @@ -/* $OpenBSD: video_if.h,v 1.13 2008/06/13 05:00:32 mglocker Exp $ */ +/* $OpenBSD: video_if.h,v 1.14 2008/07/31 15:26:25 mglocker Exp $ */ /* * Copyright (c) 2008 Robert Nagy * Copyright (c) 2008 Marcus Glocker @@ -52,7 +52,7 @@ struct video_hw_if { /* other functions */ int (*get_bufsize)(void *); - void (*start_read)(void *); + int (*start_read)(void *); }; struct video_attach_args {