Bug 16256

Comment 1 Nick Hibma 2000-01-21 18:22:10 UTC

Responsible Changed
From-To: freebsd-bugs->n_hibma

USB is my department (someone said). 
. 

Please try the following patch, it'll refuse the set config if there is
an endpoint open.

And there is some extra bits at the bottom to kill the vnodes for the
control endpoint as is already done for all the other endpoints. But
that should not make a difference.

Nick

Index: ugen.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.33
diff -u -w -r1.33 ugen.c
--- ugen.c	2000/01/20 22:05:30	1.33
+++ ugen.c	2000/01/21 18:49:00
@@ -204,6 +204,14 @@
 
 	DPRINTFN(1,("ugen_set_config: %s to configno %d, sc=%p\n",
 		    USBDEVNAME(sc->sc_dev), configno, sc));
+
+	/* We start at 1, not 0, because we don't care whether the
+	 * control endpoint is open or not. It is always present.
+	 */
+	for (endptno = 1; endptno < USB_MAX_ENDPOINTS; endptno++)
+		if (sc->sc_is_open[endptno])
+			return (USBD_IN_USE);
+
 	if (usbd_get_config_descriptor(dev)->bConfigurationValue != configno) {
 		/* Avoid setting the current value. */
 		err = usbd_set_config_no(dev, configno, 0);
@@ -688,8 +696,14 @@
 	mn = self->dv_unit * USB_MAX_ENDPOINTS;
 	vdevgone(maj, mn, mn + USB_MAX_ENDPOINTS - 1, VCHR);
 #elif defined(__FreeBSD__)
+	/* destroy the device for the control endpoint */
 	dev = makedev(UGEN_CDEV_MAJOR, UGENMINOR(USBDEVUNIT(sc->sc_dev), 0));
+	vp = SLIST_FIRST(&dev->si_hlist);
+	if (vp)
+		VOP_REVOKE(vp, REVOKEALL);
 	destroy_dev(dev);
+
+	/* destroy all devices for the other (existing) endpoints as well */
 	for (endptno = 1; endptno < USB_MAX_ENDPOINTS; endptno++) {
 		if (sc->sc_endpoints[endptno][IN].sc != NULL ||
 		    sc->sc_endpoints[endptno][OUT].sc != NULL ) {
@@ -943,7 +957,9 @@
 		if (!(flag & FWRITE))
 			return (EPERM);
 		err = ugen_set_config(sc, *(int *)addr);
-		if (err)
+		if (err == USBD_IN_USE)
+			return(EBUSY);
+		else
 			return (EIO);
 		break;
 	case USB_GET_ALTINTERFACE:


On Fri, 21 Jan 2000, Louis Mamakos wrote:

> 
> >Number:         16256
> >Category:       kern
> >Synopsis:       USB stack panic on null pointer dereference
> >Confidential:   no
> >Severity:       serious
> >Priority:       medium
> >Responsible:    freebsd-bugs
> >State:          open
> >Quarter:        
> >Keywords:       
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Fri Jan 21 07:40:01 PST 2000
> >Closed-Date:
> >Last-Modified:
> >Originator:     Louis Mamakos
> >Release:        FreeBSD 4.0-CURRENT i386
> >Organization:
> >Environment:
> 
> 	4.0-current, using ugen device driver
> 
> >Description:
> 
> 	See PR kern/16168 for details.  Essentially, if a set configuration
> ioctl() is perform on /dev/ugenX.0, while any other endpoints are open
> (e.g., /dev/ugenX.2), subsequent I/O on the other endpoints will likely
> cause a null pointer dereference.
> 
> >How-To-Repeat:
> 
> 	as above
> 
> >Fix:
> 
> 	This is just a work-around.  As discussed in the previous PR,
> the "right" answer depends on what the semantics of the driver need to be.
> Possibly device reconfiguration requests shouldn't be allowed if other
> endpoints are open?
> 
> The work-around simply removes #ifdef DIAGNOSTIC around some existing
> tests to bail out early.
> 
> Index: ugen.c
> ===================================================================
> RCS file: /usr/local/FreeBSD/cvs/src/sys/dev/usb/ugen.c,v
> retrieving revision 1.33
> diff -u -r1.33 ugen.c
> --- ugen.c	2000/01/20 22:05:30	1.33
> +++ ugen.c	2000/01/21 15:28:15
> @@ -435,16 +435,15 @@
>  	if (endpt == USB_CONTROL_ENDPOINT)
>  		return (ENODEV);
>  
> -#ifdef DIAGNOSTIC
>  	if (sce->edesc == NULL) {
>  		printf("ugenread: no edesc\n");
>  		return (EIO);
>  	}
> +
>  	if (sce->pipeh == NULL) {
>  		printf("ugenread: no pipe\n");
>  		return (EIO);
>  	}
> -#endif
>  
>  	switch (sce->edesc->bmAttributes & UE_XFERTYPE) {
>  	case UE_INTERRUPT:
> @@ -559,7 +558,6 @@
>  	if (endpt == USB_CONTROL_ENDPOINT)
>  		return (ENODEV);
>  
> -#ifdef DIAGNOSTIC
>  	if (sce->edesc == NULL) {
>  		printf("ugenwrite: no edesc\n");
>  		return (EIO);
> @@ -568,7 +566,6 @@
>  		printf("ugenwrite: no pipe\n");
>  		return (EIO);
>  	}
> -#endif
>  
>  	switch (sce->edesc->bmAttributes & UE_XFERTYPE) {
>  	case UE_BULK:
> @@ -897,12 +894,12 @@
>  		sce = &sc->sc_endpoints[endpt][IN];
>  		if (sce == NULL)
>  			return (EINVAL);
> -#ifdef DIAGNOSTIC
> +
>  		if (sce->pipeh == NULL) {
>  			printf("ugenioctl: USB_SET_SHORT_XFER, no pipe\n");
>  			return (EIO);
>  		}
> -#endif
> +
>  		if (*(int *)addr)
>  			sce->state |= UGEN_SHORT_OK;
>  		else
> @@ -912,12 +909,12 @@
>  		sce = &sc->sc_endpoints[endpt][IN];
>  		if (sce == NULL)
>  			return (EINVAL);
> -#ifdef DIAGNOSTIC
> +
>  		if (sce->pipeh == NULL) {
>  			printf("ugenioctl: USB_SET_TIMEOUT, no pipe\n");
>  			return (EIO);
>  		}
> -#endif
> +
>  		sce->timeout = *(int *)addr;
>  		return (0);
>  	default:
> @@ -1177,7 +1174,7 @@
>  	sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
>  	if (sce == NULL)
>  		return (EINVAL);
> -#ifdef DIAGNOSTIC
> +
>  	if (!sce->edesc) {
>  		printf("ugenwrite: no edesc\n");
>  		return (EIO);
> @@ -1186,7 +1183,7 @@
>  		printf("ugenpoll: no pipe\n");
>  		return (EIO);
>  	}
> -#endif
> +
>  	s = splusb();
>  	switch (sce->edesc->bmAttributes & UE_XFERTYPE) {
>  	case UE_INTERRUPT:
> 
> 
> 
> >Release-Note:
> >Audit-Trail:
> >Unformatted:
> 
> 
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-bugs" in the body of the message
> 

--
n_hibma@webweaving.org
n_hibma@freebsd.org                                          USB project
http://www.etla.net/~n_hibma/

Comment 3 Nick Hibma 2000-01-23 15:49:01 UTC

State Changed
From-To: open->closed

Fixed in rev. 1.35 of ugen.c