Re: [libusb] [PATCH] USB: usbfs: allow URBs to be reaped after disconnection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 29-01-15 17:29, Alan Stern wrote:
The usbfs API has a peculiar hole: Users are not allowed to reap their
URBs after the device has been disconnected.  There doesn't seem to be
any good reason for this; it is an ad-hoc inconsistency.

The patch allows users to issue the USBDEVFS_REAPURB and
USBDEVFS_REAPURBNDELAY ioctls (together with their 32-bit counterparts
on 64-bit systems) even after the device is gone.  If no URBs are
pending for a disconnected device then the ioctls will return -ENODEV
rather than -EAGAIN, because obviously no new URBs will ever be able
to complete.

The patch also adds a new capability flag for
USBDEVFS_GET_CAPABILITIES to indicate that the reap-after-disconnect
feature is supported.

Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Tested-by: Chris Dickens <christopher.a.dickens@xxxxxxxxx>

Patch looks good and I agree that this is something which we want to fix:

Acked-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Regards,

Hans


---


[as1773]


  drivers/usb/core/devio.c          |   63 ++++++++++++++++++++++----------------
  include/uapi/linux/usbdevice_fs.h |    3 +
  2 files changed, 39 insertions(+), 27 deletions(-)

Index: usb-3.19/include/uapi/linux/usbdevice_fs.h
===================================================================
--- usb-3.19.orig/include/uapi/linux/usbdevice_fs.h
+++ usb-3.19/include/uapi/linux/usbdevice_fs.h
@@ -128,11 +128,12 @@ struct usbdevfs_hub_portinfo {
  	char port [127];	/* e.g. port 3 connects to device 27 */
  };

-/* Device capability flags */
+/* System and bus capability flags */
  #define USBDEVFS_CAP_ZERO_PACKET		0x01
  #define USBDEVFS_CAP_BULK_CONTINUATION		0x02
  #define USBDEVFS_CAP_NO_PACKET_SIZE_LIM		0x04
  #define USBDEVFS_CAP_BULK_SCATTER_GATHER	0x08
+#define USBDEVFS_CAP_REAP_AFTER_DISCONNECT	0x10

  /* USBDEVFS_DISCONNECT_CLAIM flags & struct */

Index: usb-3.19/drivers/usb/core/devio.c
===================================================================
--- usb-3.19.orig/drivers/usb/core/devio.c
+++ usb-3.19/drivers/usb/core/devio.c
@@ -1689,7 +1689,7 @@ static struct async *reap_as(struct usb_
  	for (;;) {
  		__set_current_state(TASK_INTERRUPTIBLE);
  		as = async_getcompleted(ps);
-		if (as)
+		if (as || !connected(ps))
  			break;
  		if (signal_pending(current))
  			break;
@@ -1712,7 +1712,7 @@ static int proc_reapurb(struct usb_dev_s
  	}
  	if (signal_pending(current))
  		return -EINTR;
-	return -EIO;
+	return -ENODEV;
  }

  static int proc_reapurbnonblock(struct usb_dev_state *ps, void __user *arg)
@@ -1721,10 +1721,11 @@ static int proc_reapurbnonblock(struct u
  	struct async *as;

  	as = async_getcompleted(ps);
-	retval = -EAGAIN;
  	if (as) {
  		retval = processcompl(as, (void __user * __user *)arg);
  		free_async(as);
+	} else {
+		retval = (connected(ps) ? -EAGAIN : -ENODEV);
  	}
  	return retval;
  }
@@ -1854,7 +1855,7 @@ static int proc_reapurb_compat(struct us
  	}
  	if (signal_pending(current))
  		return -EINTR;
-	return -EIO;
+	return -ENODEV;
  }

  static int proc_reapurbnonblock_compat(struct usb_dev_state *ps, void __user *arg)
@@ -1862,11 +1863,12 @@ static int proc_reapurbnonblock_compat(s
  	int retval;
  	struct async *as;

-	retval = -EAGAIN;
  	as = async_getcompleted(ps);
  	if (as) {
  		retval = processcompl_compat(as, (void __user * __user *)arg);
  		free_async(as);
+	} else {
+		retval = (connected(ps) ? -EAGAIN : -ENODEV);
  	}
  	return retval;
  }
@@ -2038,7 +2040,8 @@ static int proc_get_capabilities(struct
  {
  	__u32 caps;

-	caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM;
+	caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM |
+			USBDEVFS_CAP_REAP_AFTER_DISCONNECT;
  	if (!ps->dev->bus->no_stop_on_short)
  		caps |= USBDEVFS_CAP_BULK_CONTINUATION;
  	if (ps->dev->bus->sg_tablesize)
@@ -2138,6 +2141,32 @@ static long usbdev_do_ioctl(struct file
  		return -EPERM;

  	usb_lock_device(dev);
+
+	/* Reap operations are allowed even after disconnection */
+	switch (cmd) {
+	case USBDEVFS_REAPURB:
+		snoop(&dev->dev, "%s: REAPURB\n", __func__);
+		ret = proc_reapurb(ps, p);
+		goto done;
+
+	case USBDEVFS_REAPURBNDELAY:
+		snoop(&dev->dev, "%s: REAPURBNDELAY\n", __func__);
+		ret = proc_reapurbnonblock(ps, p);
+		goto done;
+
+#ifdef CONFIG_COMPAT
+	case USBDEVFS_REAPURB32:
+		snoop(&dev->dev, "%s: REAPURB32\n", __func__);
+		ret = proc_reapurb_compat(ps, p);
+		goto done;
+
+	case USBDEVFS_REAPURBNDELAY32:
+		snoop(&dev->dev, "%s: REAPURBNDELAY32\n", __func__);
+		ret = proc_reapurbnonblock_compat(ps, p);
+		goto done;
+#endif
+	}
+
  	if (!connected(ps)) {
  		usb_unlock_device(dev);
  		return -ENODEV;
@@ -2231,16 +2260,6 @@ static long usbdev_do_ioctl(struct file
  			inode->i_mtime = CURRENT_TIME;
  		break;

-	case USBDEVFS_REAPURB32:
-		snoop(&dev->dev, "%s: REAPURB32\n", __func__);
-		ret = proc_reapurb_compat(ps, p);
-		break;
-
-	case USBDEVFS_REAPURBNDELAY32:
-		snoop(&dev->dev, "%s: REAPURBNDELAY32\n", __func__);
-		ret = proc_reapurbnonblock_compat(ps, p);
-		break;
-
  	case USBDEVFS_IOCTL32:
  		snoop(&dev->dev, "%s: IOCTL32\n", __func__);
  		ret = proc_ioctl_compat(ps, ptr_to_compat(p));
@@ -2252,16 +2271,6 @@ static long usbdev_do_ioctl(struct file
  		ret = proc_unlinkurb(ps, p);
  		break;

-	case USBDEVFS_REAPURB:
-		snoop(&dev->dev, "%s: REAPURB\n", __func__);
-		ret = proc_reapurb(ps, p);
-		break;
-
-	case USBDEVFS_REAPURBNDELAY:
-		snoop(&dev->dev, "%s: REAPURBNDELAY\n", __func__);
-		ret = proc_reapurbnonblock(ps, p);
-		break;
-
  	case USBDEVFS_DISCSIGNAL:
  		snoop(&dev->dev, "%s: DISCSIGNAL\n", __func__);
  		ret = proc_disconnectsignal(ps, p);
@@ -2304,6 +2313,8 @@ static long usbdev_do_ioctl(struct file
  		ret = proc_free_streams(ps, p);
  		break;
  	}
+
+ done:
  	usb_unlock_device(dev);
  	if (ret >= 0)
  		inode->i_atime = CURRENT_TIME;


------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
libusb-devel mailing list
libusb-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/libusb-devel

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux