PERFORCE change 153026 for review

[Hans Petter Selasky]

http://perforce.freebsd.org/chv.cgi?CH=153026

Change 153026 by hselasky at hselasky_laptop001 on 2008/11/16 09:57:01

	
	Fix a race freeing the generic USB transfers when
	setting the configuration.

Affected files ...

.. //depot/projects/usb/src/lib/libusb20/libusb20_ugen20.c#11 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_device.c#32 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_generic.c#33 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_generic.h#8 edit

Differences ...

==== //depot/projects/usb/src/lib/libusb20/libusb20_ugen20.c#11 (text+ko) ====

@@ -307,10 +307,27 @@
 	return (0);			/* success */
 }
 
+static void
+ugen20_tr_release(struct libusb20_device *pdev)
+{
+	struct usb2_fs_uninit fs_uninit;
+
+	if (pdev->nTransfer == 0) {
+		return;
+	}
+	/* release all pending USB transfers */
+	if (pdev->privBeData != NULL) {
+		memset(&fs_uninit, 0, sizeof(fs_uninit));
+		if (ioctl(pdev->file, USB_FS_UNINIT, &fs_uninit)) {
+			/* ignore any errors of this kind */
+		}
+	}
+	return;
+}
+
 static int
 ugen20_tr_renew(struct libusb20_device *pdev)
 {
-	struct usb2_fs_uninit fs_uninit;
 	struct usb2_fs_init fs_init;
 	struct usb2_fs_endpoint *pfse;
 	int error;
@@ -325,12 +342,7 @@
 	}
 	size = nMaxTransfer * sizeof(*pfse);
 
-	if (pdev->privBeData != NULL) {
-		memset(&fs_uninit, 0, sizeof(fs_uninit));
-		if (ioctl(pdev->file, USB_FS_UNINIT, &fs_uninit)) {
-			/* ignore any errors of this kind */
-		}
-	} else {
+	if (pdev->privBeData == NULL) {
 		pfse = malloc(size);
 		if (pfse == NULL) {
 			error = LIBUSB20_ERROR_NO_MEM;
@@ -338,7 +350,6 @@
 		}
 		pdev->privBeData = pfse;
 	}
-
 	/* reset endpoint data */
 	memset(pdev->privBeData, 0, size);
 
@@ -509,6 +520,9 @@
 {
 	int temp = cfg_index;
 
+	/* release all active USB transfers */
+	ugen20_tr_release(pdev);
+
 	if (ioctl(pdev->file_ctrl, USB_SET_CONFIG, &temp)) {
 		return (LIBUSB20_ERROR_OTHER);
 	}
@@ -548,6 +562,9 @@
 	alt_iface.uai_interface_index = iface_index;
 	alt_iface.uai_alt_index = alt_index;
 
+	/* release all active USB transfers */
+	ugen20_tr_release(pdev);
+
 	if (ioctl(pdev->file_ctrl, USB_SET_ALTINTERFACE, &alt_iface)) {
 		return (LIBUSB20_ERROR_OTHER);
 	}
@@ -559,6 +576,9 @@
 {
 	int temp = 0;
 
+	/* release all active USB transfers */
+	ugen20_tr_release(pdev);
+
 	if (ioctl(pdev->file_ctrl, USB_DEVICEENUMERATE, &temp)) {
 		return (LIBUSB20_ERROR_OTHER);
 	}

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_device.c#32 (text+ko) ====

@@ -2114,38 +2114,23 @@
 				 */
 				continue;
 			}
-			if (f->dev_ep_index == 0) {
-				/*
-				 * Don't free the generic control endpoint
-				 * yet and make sure that any USB-FS
-				 * activity is stopped!
-				 */
-				if (ugen_fs_uninit(f)) {
-					/* ignore any failures */
-					DPRINTFN(10, "udev=%p ugen_fs_uninit() "
-					    "failed! (ignored)\n", udev);
-				}
+			if ((f->dev_ep_index == 0) &&
+			    (f->fs_xfer == NULL)) {
+				/* no need to free this FIFO */
 				continue;
 			}
 		} else if (iface_index == USB_IFACE_INDEX_ANY) {
 			if ((f->methods == &usb2_ugen_methods) &&
-			    (f->dev_ep_index == 0) && (flag == 0)) {
-				/*
-				 * Don't free the generic control endpoint
-				 * yet, but make sure that any USB-FS
-				 * activity is stopped!
-				 */
-				if (ugen_fs_uninit(f)) {
-					/* ignore any failures */
-					DPRINTFN(10, "udev=%p ugen_fs_uninit() "
-					    "failed! (ignored)\n", udev);
-				}
+			    (f->dev_ep_index == 0) && (flag == 0) &&
+			    (f->fs_xfer == NULL)) {
+				/* no need to free this FIFO */
 				continue;
 			}
 		} else {
+			/* no need to free this FIFO */
 			continue;
 		}
-		/* free the FIFO */
+		/* free this FIFO */
 		usb2_fifo_free(f);
 	}
 	return;

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_generic.c#33 (text+ko) ====

@@ -83,7 +83,7 @@
 static int ugen_re_enumerate(struct usb2_fifo *f);
 static int ugen_iface_ioctl(struct usb2_fifo *f, u_long cmd, void *addr, int fflags);
 static uint8_t ugen_fs_get_complete(struct usb2_fifo *f, uint8_t *pindex);
-
+static int ugen_fs_uninit(struct usb2_fifo *f);
 
 /* structures */
 
@@ -192,6 +192,7 @@
 
 	if (ugen_fs_uninit(f)) {
 		/* ignore any errors - we are closing */
+		DPRINTFN(6, "no FIFOs\n");
 	}
 	return;
 }
@@ -591,6 +592,12 @@
 		/* no change needed */
 		return (0);
 	}
+	/* make sure all FIFO's are gone */
+	/* else there can be a deadlock */
+	if (ugen_fs_uninit(f)) {
+		/* ignore any errors */
+		DPRINTFN(6, "no FIFOs\n");
+	}
 	/* change setting - will free generic FIFOs, if any */
 	if (usb2_set_config_index(f->udev, index)) {
 		return (EIO);
@@ -612,6 +619,12 @@
 		/* not possible in device side mode */
 		return (ENOTTY);
 	}
+	/* make sure all FIFO's are gone */
+	/* else there can be a deadlock */
+	if (ugen_fs_uninit(f)) {
+		/* ignore any errors */
+		DPRINTFN(6, "no FIFOs\n");
+	}
 	/* change setting - will free generic FIFOs, if any */
 	if (usb2_set_alt_interface_index(f->udev, iface_index, alt_index)) {
 		return (EIO);

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_generic.h#8 (text+ko) ====

@@ -29,6 +29,5 @@
 
 extern struct usb2_fifo_methods usb2_ugen_methods;
 int	ugen_do_request(struct usb2_fifo *f, struct usb2_ctl_request *ur);
-int	ugen_fs_uninit(struct usb2_fifo *f);
 
 #endif					/* _USB2_GENERIC_H_ */