[Libusb-devel] [PATCH 4/4] cancel URBs in reverse order

[Alan S. <st...@ro...>]

This patch changes the way URBs are cancelled.  In a multi-URB
transfer, URBs should be cancelled in reverse order of submission.
This prevents races that might otherwise occur (after URB N is
cancelled, data may be transferred for URB N+1 before it too can be
cancelled).

---

Index: libusb/libusb/os/linux_usbfs.c
===================================================================
--- libusb.orig/libusb/os/linux_usbfs.c
+++ libusb/libusb/os/linux_usbfs.c
@@ -1453,7 +1453,10 @@ static int submit_bulk_transfer(struct u
 			if (COMPLETED_EARLY == tpriv->reap_action)
 				return 0;
 
-			for (j = 0; j < i; j++) {
+			/* The URBs are discarded in reverse order of submission, in order
+			 * avoid races.
+			 */
+			for (j = i-1; j >= 0; j--) {
 				int tmp = ioctl(dpriv->fd, IOCTL_USBFS_DISCARDURB, &urbs[j]);
 				if (tmp && errno != EINVAL)
 					usbi_warn(TRANSFER_CTX(transfer),
@@ -1610,7 +1613,7 @@ static int submit_iso_transfer(struct us
 			/* The URBs we haven't submitted yet we count as already
 			 * retired. */
 			tpriv->num_retired = num_urbs - i;
-			for (j = 0; j < i; j++) {
+			for (j = i-1; j >= 0; j--) {
 				int tmp = ioctl(dpriv->fd, IOCTL_USBFS_DISCARDURB, urbs[j]);
 				if (tmp && errno != EINVAL)
 					usbi_warn(TRANSFER_CTX(transfer),
@@ -1733,7 +1736,7 @@ static int cancel_bulk_transfer(struct u
 	if (tpriv->reap_action != ERRORED)
 		tpriv->reap_action = CANCELLED;
 
-	for (i = 0; i < tpriv->num_urbs; i++) {
+	for (i = tpriv->num_urbs - 1; i >= 0; i--) {
 		int tmp = ioctl(dpriv->fd, IOCTL_USBFS_DISCARDURB, &tpriv->urbs[i]);
 		if (tmp && errno != EINVAL)
 			usbi_warn(TRANSFER_CTX(transfer),
@@ -1755,7 +1758,7 @@ static int cancel_iso_transfer(struct us
 		return LIBUSB_ERROR_NOT_FOUND;
 
 	tpriv->reap_action = CANCELLED;
-	for (i = 0; i < tpriv->num_urbs; i++) {
+	for (i = tpriv->num_urbs - 1; i >= 0; i--) {
 		int tmp = ioctl(dpriv->fd, IOCTL_USBFS_DISCARDURB, tpriv->iso_urbs[i]);
 		if (tmp && errno != EINVAL)
 			usbi_warn(TRANSFER_CTX(transfer),
@@ -1931,7 +1934,7 @@ cancel_remainder:
 			goto completed;
 		} else {
 			int i;
-			for (i = urb_idx + 1; i < tpriv->num_urbs; i++) {
+			for (i = tpriv->num_urbs - 1; i > urb_idx; i--) {
 				/* remaining URBs with continuation flag are automatically
 				 * cancelled by the kernel */
 				if (tpriv->urbs[i].flags & USBFS_URB_BULK_CONTINUATION)