From owner-freebsd-usb@FreeBSD.ORG Sun May 14 20:20:28 2006 Return-Path: X-Original-To: freebsd-usb@hub.freebsd.org Delivered-To: freebsd-usb@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 895A416A402 for ; Sun, 14 May 2006 20:20:28 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id DF98743D46 for ; Sun, 14 May 2006 20:20:27 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k4EKKR0E012338 for ; Sun, 14 May 2006 20:20:27 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k4EKKR2T012334; Sun, 14 May 2006 20:20:27 GMT (envelope-from gnats) Resent-Date: Sun, 14 May 2006 20:20:27 GMT Resent-Message-Id: <200605142020.k4EKKR2T012334@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-usb@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, "Anish Mistry" Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 96F7A16A403 for ; Sun, 14 May 2006 20:11:51 +0000 (UTC) (envelope-from amistry@am-productions.biz) Received: from smtp3.fuse.net (mail-out3.fuse.net [216.68.8.176]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1CC6143D48 for ; Sun, 14 May 2006 20:11:50 +0000 (GMT) (envelope-from amistry@am-productions.biz) Received: from gx5.fuse.net ([69.61.164.22]) by smtp3.fuse.net (InterMail vM.6.01.04.04 201-2131-118-104-20050224) with ESMTP id <20060514201150.IMGN12345.smtp3.fuse.net@gx5.fuse.net> for ; Sun, 14 May 2006 16:11:50 -0400 Received: from bigguy.am-productions.biz ([69.61.164.22]) by gx5.fuse.net (InterMail vG.1.02.00.02 201-2136-104-102-20041210) with ESMTP id <20060514201149.GYCY4651.gx5.fuse.net@bigguy.am-productions.biz> for ; Sun, 14 May 2006 16:11:49 -0400 Message-Id: <1147637538.57596@bigguy.am-productions.biz> Date: Sun, 14 May 2006 16:12:18 -0400 From: "Anish Mistry" To: "FreeBSD gnats submit" X-Send-Pr-Version: gtk-send-pr 0.4.7 Cc: Subject: usb/97271: Fix Multiple ugen panics X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 May 2006 20:20:28 -0000 >Number: 97271 >Category: usb >Synopsis: Fix Multiple ugen panics >Confidential: no >Severity: serious >Priority: low >Responsible: freebsd-usb >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sun May 14 20:20:27 GMT 2006 >Closed-Date: >Last-Modified: >Originator: Anish Mistry >Release: FreeBSD 6.1-RELEASE i386 >Organization: AM Productions >Environment: System: FreeBSD 6.1-RELEASE #0: Wed May 10 01:44:17 EDT 2006 amistry@bigguy.am-productions.biz:/usr/obj/usr/src/sys/BIGGUY >Description: There are several race conditions is the ugen driver that make it easy to panic the system when a device is detached during an IO operation. There are also panics when select() and poll() are used on ugen device endpoints. The attached patch fixes these issues. The patch also fixes the current PRs: usb/94311 usb/68232 Some of these issues are relevant to NetBSD and probably OpenBSD and DragonflyBSD. >How-To-Repeat: >Fix: --- ugen-multiple-panics.patch begins here --- --- /sys/dev/usb/ugen.c.orig Thu Dec 15 16:57:32 2005 +++ /sys/dev/usb/ugen.c Tue May 9 12:16:28 2006 @@ -1,4 +1,4 @@ -/* $NetBSD: ugen.c,v 1.59 2002/07/11 21:14:28 augustss Exp $ */ +/* $NetBSD: ugen.c,v 1.79 2006/03/01 12:38:13 yamt Exp $ */ /* Also already merged from NetBSD: * $NetBSD: ugen.c,v 1.61 2002/09/23 05:51:20 simonb Exp $ @@ -284,6 +284,9 @@ ugen_make_devnodes(sc); #endif + usbd_add_drv_event(USB_EVENT_DRIVER_ATTACH, sc->sc_udev, + USBDEV(sc->sc_dev)); + USB_ATTACH_SUCCESS_RETURN; } @@ -383,6 +386,7 @@ M_WAITOK); niface_cache = niface; + memset(sc->sc_endpoints, 0, sizeof sc->sc_endpoints); for (ifaceno = 0; ifaceno < niface; ifaceno++) { DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno)); err = usbd_device2interface_handle(dev, ifaceno, &iface); @@ -511,7 +515,7 @@ for (dir = OUT; dir <= IN; dir++) { if (flag & (dir == OUT ? FWRITE : FREAD)) { sce = &sc->sc_endpoints[endpt][dir]; - if (sce->edesc == 0) + if (sce == 0 ||sce->edesc == 0) return (ENXIO); } } @@ -650,7 +654,7 @@ if (!(flag & (dir == OUT ? FWRITE : FREAD))) continue; sce = &sc->sc_endpoints[endpt][dir]; - if (sce->pipeh == NULL) + if (sce == NULL || sce->pipeh == NULL) continue; DPRINTFN(5, ("ugenclose: endpt=%d dir=%d sce=%p\n", endpt, dir, sce)); @@ -835,6 +839,9 @@ USB_GET_SC(ugen, UGENUNIT(dev), sc); + if(sc->sc_dying) + return (EIO); + UGEN_DEV_REF(dev, sc); error = ugen_do_read(sc, endpt, uio, flag); UGEN_DEV_RELE(dev, sc); @@ -933,6 +940,9 @@ USB_GET_SC(ugen, UGENUNIT(dev), sc); + if (sc->sc_dying) + return (EIO); + UGEN_DEV_REF(dev, sc); error = ugen_do_write(sc, endpt, uio, flag); UGEN_DEV_RELE(dev, sc); @@ -969,8 +979,31 @@ return; USB_GET_SC(ugen, UGENUNIT(dev), sc); sce = &sc->sc_endpoints[endpt][IN]; - if (sce->pipeh) - usbd_abort_pipe(sce->pipeh); + if (sce) + { + if(sce->pipeh) + usbd_abort_pipe(sce->pipeh); + + if (sce->state & UGEN_ASLP) { + sce->state &= ~UGEN_ASLP; + DPRINTFN(5, ("ugenpurge: waking %p\n", sce)); + wakeup(sce); + } + selwakeuppri(&sce->rsel, PZERO); + } + sce = &sc->sc_endpoints[endpt][OUT]; + if (sce) + { + if(sce->pipeh) + usbd_abort_pipe(sce->pipeh); + + if (sce->state & UGEN_ASLP) { + sce->state &= ~UGEN_ASLP; + DPRINTFN(5, ("ugenpurge: waking %p\n", sce)); + wakeup(sce); + } + selwakeuppri(&sce->rsel, PZERO); + } } #endif @@ -994,11 +1027,13 @@ for (i = 0; i < USB_MAX_ENDPOINTS; i++) { for (dir = OUT; dir <= IN; dir++) { sce = &sc->sc_endpoints[i][dir]; - if (sce->pipeh) + if (sce && sce->pipeh) usbd_abort_pipe(sce->pipeh); + selwakeuppri(&sce->rsel, PZERO); } } + #if defined(__NetBSD__) || defined(__OpenBSD__) s = splusb(); if (sc->sc_refcnt > 0) { @@ -1035,6 +1070,9 @@ destroy_dev(sc->dev); #endif + usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->sc_udev, + USBDEV(sc->sc_dev)); + return (0); } @@ -1292,7 +1330,7 @@ /* This flag only affects read */ sce = &sc->sc_endpoints[endpt][IN]; - if (sce->pipeh == NULL) { + if (sce == NULL || sce->pipeh == NULL) { printf("ugenioctl: USB_SET_SHORT_XFER, no pipe\n"); return (EIO); } @@ -1304,6 +1342,12 @@ return (0); case USB_SET_TIMEOUT: sce = &sc->sc_endpoints[endpt][IN]; + if (sce == NULL + /* XXX this shouldn't happen, but the distinction between + input and output pipes isn't clear enough. + || sce->pipeh == NULL */ + ) + return (EINVAL); sce->timeout = *(int *)addr; return (0); default: @@ -1331,9 +1375,6 @@ err = ugen_set_config(sc, *(int *)addr); switch (err) { case USBD_NORMAL_COMPLETION: -#if defined(__FreeBSD__) - ugen_make_devnodes(sc); -#endif break; case USBD_IN_USE: return (EBUSY); @@ -1541,6 +1582,9 @@ USB_GET_SC(ugen, UGENUNIT(dev), sc); + if (sc->sc_dying) + return (EIO); + UGEN_DEV_REF(dev, sc); error = ugen_do_ioctl(sc, endpt, cmd, addr, flag, p); UGEN_DEV_RELE(dev, sc); @@ -1555,14 +1599,32 @@ int revents = 0; int s; + /* Do not allow to poll a control endpoint */ + if ( UGENENDPOINT(dev) == USB_CONTROL_ENDPOINT ) + return (EIO); + USB_GET_SC(ugen, UGENUNIT(dev), sc); if (sc->sc_dying) return (EIO); - /* XXX always IN */ - sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN]; -#ifdef DIAGNOSTIC + if((events & POLLIN) && (events & POLLOUT)) { + printf("ugenpoll: POLLIN and POLLOUT? We're not handling it, so bail.\n"); + return (EIO); + } + + if(events & (POLLIN | POLLRDNORM)) + sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN]; + else if(events & (POLLOUT | POLLWRNORM)) + sce = &sc->sc_endpoints[UGENENDPOINT(dev)][OUT]; + else { + printf("ugenpoll: unhandled input event\n"); + return (EIO); + } + + if (sce == NULL) + return (EIO); + if (!sce->edesc) { printf("ugenpoll: no edesc\n"); return (EIO); @@ -1571,23 +1633,26 @@ printf("ugenpoll: no pipe\n"); return (EIO); } -#endif s = splusb(); switch (sce->edesc->bmAttributes & UE_XFERTYPE) { case UE_INTERRUPT: - if (events & (POLLIN | POLLRDNORM)) { - if (sce->q.c_cc > 0) + if (sce->q.c_cc > 0) { + if (events & (POLLIN | POLLRDNORM)) revents |= events & (POLLIN | POLLRDNORM); - else - selrecord(p, &sce->rsel); + else if (events & (POLLOUT | POLLWRNORM)) + revents |= events & (POLLOUT | POLLWRNORM); + } else { + selrecord(p, &sce->rsel); } break; case UE_ISOCHRONOUS: - if (events & (POLLIN | POLLRDNORM)) { - if (sce->cur != sce->fill) + if (sce->cur != sce->fill) { + if (events & (POLLIN | POLLRDNORM)) revents |= events & (POLLIN | POLLRDNORM); - else - selrecord(p, &sce->rsel); + else if (events & (POLLOUT | POLLWRNORM)) + revents |= events & (POLLOUT | POLLWRNORM); + } else { + selrecord(p, &sce->rsel); } break; case UE_BULK: --- ugen-multiple-panics.patch ends here --- >Release-Note: >Audit-Trail: >Unformatted: