Add extra safety locking when clobbering xfer->flags_int.started in start and

stop functions, because xfer->flags_int is also updated by the USB controller,
under the controller lock.

Submitted by:	Hans Petter Selasky

1 files changed, 21 insertions, 7 deletions

diff --git a/sys/dev/usb/usb_transfer.c b/sys/dev/usb/usb_transfer.c
index 9a063a1..8031b845 100644
--- a/sys/dev/usb/usb_transfer.c
+++ b/sys/dev/usb/usb_transfer.c
@@ -1332,7 +1332,9 @@ usbd_setup_ctrl_transfer(struct usb_xfer *xfer)
 	/* check if there is a length mismatch */
 
 	if (len > xfer->flags_int.control_rem) {
-		DPRINTFN(0, "Length greater than remaining length!\n");
+		DPRINTFN(0, "Length (%d) greater than "
+		    "remaining length (%d)!\n", len,
+		    xfer->flags_int.control_rem);
 		goto error;
 	}
 	/* check if we are doing a short transfer */
@@ -1620,7 +1622,10 @@ usbd_transfer_start(struct usb_xfer *xfer)
 	/* mark the USB transfer started */
 
 	if (!xfer->flags_int.started) {
+		/* lock the BUS lock to avoid races updating flags_int */
+		USB_BUS_LOCK(xfer->xroot->bus);
 		xfer->flags_int.started = 1;
+		USB_BUS_UNLOCK(xfer->xroot->bus);
 	}
 	/* check if the USB transfer callback is already transferring */
 
@@ -1655,14 +1660,21 @@ usbd_transfer_stop(struct usb_xfer *xfer)
 	/* check if the USB transfer was ever opened */
 
 	if (!xfer->flags_int.open) {
-		/* nothing to do except clearing the "started" flag */
-		xfer->flags_int.started = 0;
+		if (xfer->flags_int.started) {
+			/* nothing to do except clearing the "started" flag */
+			/* lock the BUS lock to avoid races updating flags_int */
+			USB_BUS_LOCK(xfer->xroot->bus);
+			xfer->flags_int.started = 0;
+			USB_BUS_UNLOCK(xfer->xroot->bus);
+		}
 		return;
 	}
 	/* try to stop the current USB transfer */
 
 	USB_BUS_LOCK(xfer->xroot->bus);
-	xfer->error = USB_ERR_CANCELLED;/* override any previous error */
+	/* override any previous error */
+	xfer->error = USB_ERR_CANCELLED;
+
 	/*
 	 * Clear "open" and "started" when both private and USB lock
 	 * is locked so that we don't get a race updating "flags_int"
@@ -2121,9 +2133,6 @@ usb_dma_delay_done_cb(void *arg)
 
 	DPRINTFN(3, "Completed %p\n", xfer);
 
-	/* only delay once */
-	xfer->flags_int.did_dma_delay = 1;
-
 	/* queue callback for execution, again */
 	usbd_transfer_done(xfer, 0);
 }
@@ -2193,6 +2202,8 @@ usbd_transfer_done(struct usb_xfer *xfer, usb_error_t error)
 	 */
 	if (!xfer->flags_int.transferring) {
 		DPRINTF("not transferring\n");
+		/* end of control transfer, if any */
+		xfer->flags_int.control_act = 0;
 		return;
 	}
 	/* only set transfer error if not already set */
@@ -2491,6 +2502,9 @@ usbd_callback_wrapper_sub(struct usb_xfer *xfer)
 
 		usb_timeout_t temp;
 
+		/* only delay once */
+		xfer->flags_int.did_dma_delay = 1;
+
 		/* we can not cancel this delay */
 		xfer->flags_int.can_cancel_immed = 0;