Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 19 Sep 2018 07:56:20 +0000 (UTC)
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r338788 - stable/11/lib/libusb
Message-ID:  <201809190756.w8J7uK1s097271@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: hselasky
Date: Wed Sep 19 07:56:19 2018
New Revision: 338788
URL: https://svnweb.freebsd.org/changeset/base/338788

Log:
  MFC r338616:
  Fix issues about cancelling USB transfers in LibUSB when the USB device has
  been detached. When a USB device has been detached the kernel file handle
  stops responding to commands. USB applications which continue to run after
  the USB device has been detached, depend on LibUSB generated events to tear
  down its pending USB transfers. Add code to handle the needed cleanup when
  processing the USB transfer(s) fails and prevent new USB transfer(s) from
  being submitted.
  
  Found by:		Ludovic Rousseau <ludovic.rousseau+freebsd@gmail.com>
  PR:			231076
  Sponsored by:		Mellanox Technologies

Modified:
  stable/11/lib/libusb/libusb10.c
  stable/11/lib/libusb/libusb10.h
  stable/11/lib/libusb/libusb10_io.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/lib/libusb/libusb10.c
==============================================================================
--- stable/11/lib/libusb/libusb10.c	Wed Sep 19 07:10:28 2018	(r338787)
+++ stable/11/lib/libusb/libusb10.c	Wed Sep 19 07:56:19 2018	(r338788)
@@ -112,6 +112,19 @@ libusb_set_nonblocking(int f)
 	fcntl(f, F_SETFL, flags);
 }
 
+static void
+libusb10_wakeup_event_loop(libusb_context *ctx)
+{
+	uint8_t dummy = 0;
+	int err;
+
+	err = write(ctx->ctrl_pipe[1], &dummy, sizeof(dummy));
+	if (err < (int)sizeof(dummy)) {
+		/* ignore error, if any */
+		DPRINTF(ctx, LIBUSB_DEBUG_FUNCTION, "Waking up event loop failed!");
+	}
+}
+
 int
 libusb_init(libusb_context **context)
 {
@@ -482,7 +495,6 @@ libusb_open(libusb_device *dev, libusb_device_handle *
 {
 	libusb_context *ctx = dev->ctx;
 	struct libusb20_device *pdev = dev->os_priv;
-	uint8_t dummy;
 	int err;
 
 	if (devh == NULL)
@@ -504,12 +516,8 @@ libusb_open(libusb_device *dev, libusb_device_handle *
 	    POLLOUT | POLLRDNORM | POLLWRNORM);
 
 	/* make sure our event loop detects the new device */
-	dummy = 0;
-	err = write(ctx->ctrl_pipe[1], &dummy, sizeof(dummy));
-	if (err < (int)sizeof(dummy)) {
-		/* ignore error, if any */
-		DPRINTF(ctx, LIBUSB_DEBUG_FUNCTION, "libusb_open write failed!");
-	}
+	libusb10_wakeup_event_loop(ctx);
+
 	*devh = pdev;
 
 	return (0);
@@ -562,8 +570,6 @@ libusb_close(struct libusb20_device *pdev)
 {
 	libusb_context *ctx;
 	struct libusb_device *dev;
-	uint8_t dummy;
-	int err;
 
 	if (pdev == NULL)
 		return;			/* be NULL safe */
@@ -579,12 +585,7 @@ libusb_close(struct libusb20_device *pdev)
 	libusb_unref_device(dev);
 
 	/* make sure our event loop detects the closed device */
-	dummy = 0;
-	err = write(ctx->ctrl_pipe[1], &dummy, sizeof(dummy));
-	if (err < (int)sizeof(dummy)) {
-		/* ignore error, if any */
-		DPRINTF(ctx, LIBUSB_DEBUG_FUNCTION, "libusb_close write failed!");
-	}
+	libusb10_wakeup_event_loop(ctx);
 }
 
 libusb_device *
@@ -1312,7 +1313,6 @@ libusb10_submit_transfer_sub(struct libusb20_device *p
 	int buffsize;
 	int maxframe;
 	int temp;
-	uint8_t dummy;
 
 	dev = libusb_get_device(pdev);
 
@@ -1413,10 +1413,8 @@ found:
 
 failure:
 	libusb10_complete_transfer(pxfer0, sxfer, LIBUSB_TRANSFER_ERROR);
-
 	/* make sure our event loop spins the done handler */
-	dummy = 0;
-	err = write(dev->ctx->ctrl_pipe[1], &dummy, sizeof(dummy));
+	libusb10_wakeup_event_loop(dev->ctx);
 }
 
 /* The following function must be called unlocked */
@@ -1457,6 +1455,8 @@ libusb_submit_transfer(struct libusb_transfer *uxfer)
 	    (libusb20_tr_get_priv_sc1(pxfer0) == sxfer) ||
 	    (libusb20_tr_get_priv_sc1(pxfer1) == sxfer)) {
 		err = LIBUSB_ERROR_BUSY;
+	} else if (dev->device_is_gone != 0) {
+		err = LIBUSB_ERROR_NO_DEVICE;
 	} else {
 
 		/* set pending state */
@@ -1488,6 +1488,7 @@ libusb_cancel_transfer(struct libusb_transfer *uxfer)
 	struct libusb20_transfer *pxfer1;
 	struct libusb_super_transfer *sxfer;
 	struct libusb_device *dev;
+	struct libusb_device_handle *devh;
 	uint8_t endpoint;
 	int retval;
 
@@ -1495,12 +1496,12 @@ libusb_cancel_transfer(struct libusb_transfer *uxfer)
 		return (LIBUSB_ERROR_INVALID_PARAM);
 
 	/* check if not initialised */
-	if (uxfer->dev_handle == NULL)
+	if ((devh = uxfer->dev_handle) == NULL)
 		return (LIBUSB_ERROR_NOT_FOUND);
 
 	endpoint = uxfer->endpoint;
 
-	dev = libusb_get_device(uxfer->dev_handle);
+	dev = libusb_get_device(devh);
 
 	DPRINTF(dev->ctx, LIBUSB_DEBUG_FUNCTION, "libusb_cancel_transfer enter");
 
@@ -1511,8 +1512,8 @@ libusb_cancel_transfer(struct libusb_transfer *uxfer)
 
 	CTX_LOCK(dev->ctx);
 
-	pxfer0 = libusb10_get_transfer(uxfer->dev_handle, endpoint, 0);
-	pxfer1 = libusb10_get_transfer(uxfer->dev_handle, endpoint, 1);
+	pxfer0 = libusb10_get_transfer(devh, endpoint, 0);
+	pxfer1 = libusb10_get_transfer(devh, endpoint, 1);
 
 	if (sxfer->state != LIBUSB_SUPER_XFER_ST_PEND) {
 		/* only update the transfer status */
@@ -1524,23 +1525,38 @@ libusb_cancel_transfer(struct libusb_transfer *uxfer)
 		sxfer->entry.tqe_prev = NULL;
 		libusb10_complete_transfer(NULL,
 		    sxfer, LIBUSB_TRANSFER_CANCELLED);
+		/* make sure our event loop spins the done handler */
+		libusb10_wakeup_event_loop(dev->ctx);
 	} else if (pxfer0 == NULL || pxfer1 == NULL) {
 		/* not started */
 		retval = LIBUSB_ERROR_NOT_FOUND;
 	} else if (libusb20_tr_get_priv_sc1(pxfer0) == sxfer) {
 		libusb10_complete_transfer(pxfer0,
 		    sxfer, LIBUSB_TRANSFER_CANCELLED);
-		libusb20_tr_stop(pxfer0);
-		/* make sure the queue doesn't stall */
-		libusb10_submit_transfer_sub(
-		    uxfer->dev_handle, endpoint);
+		if (dev->device_is_gone != 0) {
+			/* clear transfer pointer */
+			libusb20_tr_set_priv_sc1(pxfer0, NULL);
+			/* make sure our event loop spins the done handler */
+			libusb10_wakeup_event_loop(dev->ctx);
+		} else {
+			libusb20_tr_stop(pxfer0);
+			/* make sure the queue doesn't stall */
+			libusb10_submit_transfer_sub(devh, endpoint);
+		}
 	} else if (libusb20_tr_get_priv_sc1(pxfer1) == sxfer) {
 		libusb10_complete_transfer(pxfer1,
 		    sxfer, LIBUSB_TRANSFER_CANCELLED);
-		libusb20_tr_stop(pxfer1);
-		/* make sure the queue doesn't stall */
-		libusb10_submit_transfer_sub(
-		    uxfer->dev_handle, endpoint);
+		/* check if handle is still active */
+		if (dev->device_is_gone != 0) {
+			/* clear transfer pointer */
+			libusb20_tr_set_priv_sc1(pxfer1, NULL);
+			/* make sure our event loop spins the done handler */
+			libusb10_wakeup_event_loop(dev->ctx);
+		} else {
+			libusb20_tr_stop(pxfer1);
+			/* make sure the queue doesn't stall */
+			libusb10_submit_transfer_sub(devh, endpoint);
+		}
 	} else {
 		/* not started */
 		retval = LIBUSB_ERROR_NOT_FOUND;
@@ -1566,6 +1582,35 @@ libusb10_cancel_all_transfer(libusb_device *dev)
 		if (xfer == NULL)
 			continue;
 		libusb20_tr_close(xfer);
+	}
+}
+
+UNEXPORTED void
+libusb10_cancel_all_transfer_locked(struct libusb20_device *pdev, struct libusb_device *dev)
+{
+	struct libusb_super_transfer *sxfer;
+	unsigned x;
+
+	for (x = 0; x != LIBUSB_NUM_SW_ENDPOINTS; x++) {
+		struct libusb20_transfer *xfer;
+
+		xfer = libusb20_tr_get_pointer(pdev, x);
+		if (xfer == NULL)
+			continue;
+		if (libusb20_tr_pending(xfer) == 0)
+			continue;
+		sxfer = libusb20_tr_get_priv_sc1(xfer);
+		if (sxfer == NULL)
+			continue;
+		/* complete pending transfer */
+		libusb10_complete_transfer(xfer, sxfer, LIBUSB_TRANSFER_ERROR);
+	}
+
+	while ((sxfer = TAILQ_FIRST(&dev->tr_head))) {
+		TAILQ_REMOVE(&dev->tr_head, sxfer, entry);
+
+		/* complete pending transfer */
+		libusb10_complete_transfer(NULL, sxfer, LIBUSB_TRANSFER_ERROR);
 	}
 }
 

Modified: stable/11/lib/libusb/libusb10.h
==============================================================================
--- stable/11/lib/libusb/libusb10.h	Wed Sep 19 07:10:28 2018	(r338787)
+++ stable/11/lib/libusb/libusb10.h	Wed Sep 19 07:56:19 2018	(r338788)
@@ -114,6 +114,8 @@ struct libusb_context {
 struct libusb_device {
 	int	refcnt;
 
+	int	device_is_gone;
+
 	uint32_t claimed_interfaces;
 
 	struct libusb_super_pollfd dev_poll;
@@ -132,5 +134,6 @@ extern struct libusb_context *usbi_default_context;
 void	libusb10_add_pollfd(libusb_context *ctx, struct libusb_super_pollfd *pollfd, struct libusb20_device *pdev, int fd, short events);
 void	libusb10_remove_pollfd(libusb_context *ctx, struct libusb_super_pollfd *pollfd);
 void	libusb10_cancel_all_transfer(libusb_device *dev);
+void	libusb10_cancel_all_transfer_locked(struct libusb20_device *pdev, struct libusb_device *dev);
 
 #endif					/* __LIBUSB10_H__ */

Modified: stable/11/lib/libusb/libusb10_io.c
==============================================================================
--- stable/11/lib/libusb/libusb10_io.c	Wed Sep 19 07:10:28 2018	(r338787)
+++ stable/11/lib/libusb/libusb10_io.c	Wed Sep 19 07:56:19 2018	(r338788)
@@ -159,17 +159,19 @@ libusb10_handle_events_sub(struct libusb_context *ctx,
 		if (ppdev[i] != NULL) {
 			dev = libusb_get_device(ppdev[i]);
 
-			if (fds[i].revents == 0)
-				err = 0;	/* nothing to do */
-			else
+			if (fds[i].revents != 0) {
 				err = libusb20_dev_process(ppdev[i]);
 
-			if (err) {
-				/* cancel all transfers - device is gone */
-				libusb10_cancel_all_transfer(dev);
+				if (err) {
+					/* set device is gone */
+					dev->device_is_gone = 1;
 
-				/* remove USB device from polling loop */
-				libusb10_remove_pollfd(dev->ctx, &dev->dev_poll);
+					/* remove USB device from polling loop */
+					libusb10_remove_pollfd(dev->ctx, &dev->dev_poll);
+
+					/* cancel all pending transfers */
+					libusb10_cancel_all_transfer_locked(ppdev[i], dev);
+				}
 			}
 			CTX_UNLOCK(ctx);
 			libusb_unref_device(dev);
@@ -178,10 +180,8 @@ libusb10_handle_events_sub(struct libusb_context *ctx,
 		} else {
 			uint8_t dummy;
 
-			while (1) {
-				if (read(fds[i].fd, &dummy, 1) != 1)
-					break;
-			}
+			while (read(fds[i].fd, &dummy, 1) == 1)
+				;
 		}
 	}
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201809190756.w8J7uK1s097271>