>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:
_______________________________________________
freebsd-usbfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to
"freebsd-usb-unsubscribefreebsd.org"
|