RFC: Add USBDEVFS_TRY_DISCONNECT ioctl

From: Alan Stern
Date: Wed Aug 24 2011 - 16:32:35 EST

On Sun, 21 Aug 2011, Greg KH wrote:

> On Fri, Aug 19, 2011 at 10:56:23PM -0400, Alan Stern wrote:
> > > It's of course racey for userspace to check
> > > whether a device is busy and then disconnect the driver, but the "try
> > > disconnect" ioctl could cause the driver to disconnect itself. In the end there
> > > wasn't a very good solution to this problem.
> >
> > In principle it's quite doable. The question is whether people will
> > stand for it. Greg KH has already come out against the idea, although
> > perhaps he could be persuaded. It doesn't help that this new mechanism
> > would be used only for USB; if other subsystems could benefit from it
> > too then it would be easier to sell.
>
> I still don't think this would really work that well. But it wouldn't
> be just USB devices that need it, in the future we will have lots of
> detatched storage devices that guests want access to in "native" mode
> (Thunderbolt being one good example.)
>
> But feel free to change my mind with a patch showing just how this all
> would work :)

Okay, here's a sample patch. Actually it's three patches, listed one
after another, but people can apply it like a single patch.

1. Introduce the USBDEVFS_TRY_DISCONNECT ioctl and the check_busy
callback it uses. Implement the callback in the usbfs driver;
this gives a way for programs to unbind kernel drivers without
unbinding other userspace drivers.

2. Implement device-file reference tracking in the SCSI layer,
and the device_open and device_close callbacks it uses.

3. Implement both check_busy and device_{open,close} in
usb-storage.

Attached is a sample program that demonstrates how to use this. It
will unbind usb-storage from a USB interface if the storage device
isn't open or mounted, otherwise it will fail with a "device busy"
error. It will also unbind any other USB kernel driver from an
interface -- but it won't unbind a usbfs userspace driver that has
claimed an interface.

Alan Stern



Patch 1: USBDEVFS_TRY_DISCONNECT

drivers/usb/core/devio.c | 13 +++++++++++++
include/linux/usb.h | 5 +++++
include/linux/usbdevice_fs.h | 1 +
3 files changed, 19 insertions(+)

Index: usb-3.1/include/linux/usbdevice_fs.h
===================================================================
--- usb-3.1.orig/include/linux/usbdevice_fs.h
+++ usb-3.1/include/linux/usbdevice_fs.h
@@ -204,4 +204,5 @@ struct usbdevfs_ioctl32 {
#define USBDEVFS_CONNECT _IO('U', 23)
#define USBDEVFS_CLAIM_PORT _IOR('U', 24, unsigned int)
#define USBDEVFS_RELEASE_PORT _IOR('U', 25, unsigned int)
+#define USBDEVFS_TRY_DISCONNECT _IO('U', 26)
#endif /* _LINUX_USBDEVICE_FS_H */
Index: usb-3.1/include/linux/usb.h
===================================================================
--- usb-3.1.orig/include/linux/usb.h
+++ usb-3.1/include/linux/usb.h
@@ -812,6 +812,9 @@ struct usbdrv_wrap {
* post_reset method is called.
* @post_reset: Called by usb_reset_device() after the device
* has been reset
+ * @check_busy: Called to see whether the device is currently
+ * busy before doing a user-initiated unbind. The driver must not
+ * allow the device to become busy after this routine returns 0.
* @id_table: USB drivers use ID table to support hotplugging.
* Export this with MODULE_DEVICE_TABLE(usb,...). This must be set
* or your driver's probe function will never get called.
@@ -858,6 +861,8 @@ struct usb_driver {
int (*pre_reset)(struct usb_interface *intf);
int (*post_reset)(struct usb_interface *intf);

+ int (*check_busy)(struct usb_interface *intf);
+
const struct usb_device_id *id_table;

struct usb_dynids dynids;
Index: usb-3.1/drivers/usb/core/devio.c
===================================================================
--- usb-3.1.orig/drivers/usb/core/devio.c
+++ usb-3.1/drivers/usb/core/devio.c
@@ -516,12 +516,18 @@ static int driver_resume(struct usb_inte
return 0;
}

+static int driver_check_busy(struct usb_interface *intf)
+{
+ return -EBUSY;
+}
+
struct usb_driver usbfs_driver = {
.name = "usbfs",
.probe = driver_probe,
.disconnect = driver_disconnect,
.suspend = driver_suspend,
.resume = driver_resume,
+ .check_busy = driver_check_busy,
};

static int claimintf(struct dev_state *ps, unsigned int ifnum)
@@ -1647,9 +1653,16 @@ static int proc_ioctl(struct dev_state *
else switch (ctl->ioctl_code) {

/* disconnect kernel driver from interface */
+ case USBDEVFS_TRY_DISCONNECT:
case USBDEVFS_DISCONNECT:
if (intf->dev.driver) {
driver = to_usb_driver(intf->dev.driver);
+ if (ctl->ioctl_code == USBDEVFS_TRY_DISCONNECT &&
+ driver->check_busy) {
+ retval = driver->check_busy(intf);
+ if (retval)
+ break;
+ }
dev_dbg(&intf->dev, "disconnect by usbfs\n");
usb_driver_release_interface(driver, intf);
} else



Patch 2: SCSI device file open reference tracking

drivers/scsi/hosts.c | 32 ++++++++++++++++++++++++++++++++
drivers/scsi/sd.c | 7 +++++++
drivers/scsi/sg.c | 8 +++++++-
drivers/scsi/sr.c | 3 ++-
include/scsi/scsi_device.h | 2 ++
include/scsi/scsi_host.h | 19 +++++++++++++++++++
6 files changed, 69 insertions(+), 2 deletions(-)

Index: usb-3.1/include/scsi/scsi_host.h
===================================================================
--- usb-3.1.orig/include/scsi/scsi_host.h
+++ usb-3.1/include/scsi/scsi_host.h
@@ -356,6 +356,25 @@ struct scsi_host_template {
enum blk_eh_timer_return (*eh_timed_out)(struct scsi_cmnd *);

/*
+ * This function is called when a device file is about to be opened;
+ * if the function returns nonzero then the open fails. The low
+ * level driver can use this to track the number of open device file
+ * references (including mounts), and thereby know when it is safe to
+ * unregister the host without disrupting mounted filesystems or
+ * the like.
+ *
+ * Status: OPTIONAL
+ */
+ int (*device_open)(struct Scsi_Host *, struct scsi_device *);
+
+ /*
+ * This function is called after a device file is closed.
+ *
+ * Status: OPTIONAL
+ */
+ void (*device_close)(struct Scsi_Host *, struct scsi_device *);
+
+ /*
* Name of proc directory
*/
const char *proc_name;
Index: usb-3.1/include/scsi/scsi_device.h
===================================================================
--- usb-3.1.orig/include/scsi/scsi_device.h
+++ usb-3.1/include/scsi/scsi_device.h
@@ -361,6 +361,8 @@ extern struct scsi_event *sdev_evt_alloc
extern void sdev_evt_send(struct scsi_device *sdev, struct scsi_event *evt);
extern void sdev_evt_send_simple(struct scsi_device *sdev,
enum scsi_device_event evt_type, gfp_t gfpflags);
+extern int scsi_device_open(struct scsi_device *sdev);
+extern void scsi_device_close(struct scsi_device *sdev);
extern int scsi_device_quiesce(struct scsi_device *sdev);
extern void scsi_device_resume(struct scsi_device *sdev);
extern void scsi_target_quiesce(struct scsi_target *);
Index: usb-3.1/drivers/scsi/hosts.c
===================================================================
--- usb-3.1.orig/drivers/scsi/hosts.c
+++ usb-3.1/drivers/scsi/hosts.c
@@ -577,3 +577,35 @@ void scsi_flush_work(struct Scsi_Host *s
flush_workqueue(shost->work_q);
}
EXPORT_SYMBOL_GPL(scsi_flush_work);
+
+/**
+ * scsi_device_open - Check whether it's okay to open a device file
+ * @sdev: Device whose file will be opened.
+ *
+ * Return value:
+ * 0 if open is allowed; negative errno otherwise
+ **/
+int scsi_device_open(struct scsi_device *sdev)
+{
+ struct Scsi_Host *shost = sdev->host;
+ struct scsi_host_template *sht = shost->hostt;
+
+ if (!sht->device_open)
+ return 0;
+ return sht->device_open(shost, sdev);
+}
+EXPORT_SYMBOL_GPL(scsi_device_open);
+
+/**
+ * scsi_device_close - Announce that a device file has been closed
+ * @sdev: Device whose file has been closed.
+ **/
+void scsi_device_close(struct scsi_device *sdev)
+{
+ struct Scsi_Host *shost = sdev->host;
+ struct scsi_host_template *sht = shost->hostt;
+
+ if (sht->device_close)
+ sht->device_close(shost, sdev);
+}
+EXPORT_SYMBOL_GPL(scsi_device_close);
Index: usb-3.1/drivers/scsi/sd.c
===================================================================
--- usb-3.1.orig/drivers/scsi/sd.c
+++ usb-3.1/drivers/scsi/sd.c
@@ -935,6 +935,10 @@ static int sd_open(struct block_device *

sdev = sdkp->device;

+ retval = scsi_device_open(sdev);
+ if (retval)
+ goto error_open;
+
retval = scsi_autopm_get_device(sdev);
if (retval)
goto error_autopm;
@@ -985,6 +989,8 @@ static int sd_open(struct block_device *
error_out:
scsi_autopm_put_device(sdev);
error_autopm:
+ scsi_device_close(sdev);
+error_open:
scsi_disk_put(sdkp);
return retval;
}
@@ -1020,6 +1026,7 @@ static int sd_release(struct gendisk *di
*/

scsi_autopm_put_device(sdev);
+ scsi_device_close(sdev);
scsi_disk_put(sdkp);
return 0;
}
Index: usb-3.1/drivers/scsi/sr.c
===================================================================
--- usb-3.1.orig/drivers/scsi/sr.c
+++ usb-3.1/drivers/scsi/sr.c
@@ -623,7 +623,7 @@ static int sr_open(struct cdrom_device_i
if (!scsi_block_when_processing_errors(sdev))
goto error_out;

- return 0;
+ retval = scsi_device_open(sdev);

error_out:
return retval;
@@ -636,6 +636,7 @@ static void sr_release(struct cdrom_devi
if (cd->device->sector_size > 2048)
sr_set_blocklength(cd, 2048);

+ scsi_device_close(cd->device);
}

static int sr_probe(struct device *dev)
Index: usb-3.1/drivers/scsi/sg.c
===================================================================
--- usb-3.1.orig/drivers/scsi/sg.c
+++ usb-3.1/drivers/scsi/sg.c
@@ -247,10 +247,14 @@ sg_open(struct inode *inode, struct file
if (retval)
goto sg_put;

- retval = scsi_autopm_get_device(sdp->device);
+ retval = scsi_device_open(sdp->device);
if (retval)
goto sdp_put;

+ retval = scsi_autopm_get_device(sdp->device);
+ if (retval)
+ goto scsi_device_close;
+
if (!((flags & O_NONBLOCK) ||
scsi_block_when_processing_errors(sdp->device))) {
retval = -ENXIO;
@@ -310,6 +314,8 @@ sg_open(struct inode *inode, struct file
error_out:
if (retval) {
scsi_autopm_put_device(sdp->device);
+scsi_device_close:
+ scsi_device_close(sdp->device);
sdp_put:
scsi_device_put(sdp->device);
}



Patch 3: implementation for usb-storage

drivers/usb/storage/scsiglue.c | 30 ++++++++++++++++++++++++++++++
drivers/usb/storage/usb.c | 21 +++++++++++++++++++++
drivers/usb/storage/usb.h | 1 +
3 files changed, 52 insertions(+)

Index: usb-3.1/drivers/usb/storage/usb.h
===================================================================
--- usb-3.1.orig/drivers/usb/storage/usb.h
+++ usb-3.1/drivers/usb/storage/usb.h
@@ -135,6 +135,7 @@ struct us_data {
struct scsi_cmnd *srb; /* current srb */
unsigned int tag; /* current dCBWTag */
char scsi_name[32]; /* scsi_host name */
+ int open_count; /* open file refs */

/* control and bulk communications data */
struct urb *current_urb; /* USB requests */
Index: usb-3.1/drivers/usb/storage/usb.c
===================================================================
--- usb-3.1.orig/drivers/usb/storage/usb.c
+++ usb-3.1/drivers/usb/storage/usb.c
@@ -217,6 +217,26 @@ int usb_stor_post_reset(struct usb_inter
}
EXPORT_SYMBOL_GPL(usb_stor_post_reset);

+int usb_stor_check_busy(struct usb_interface *iface)
+{
+ struct us_data *us = usb_get_intfdata(iface);
+ struct Scsi_Host *host = us_to_host(us);
+ int rc = -EBUSY;
+
+ US_DEBUGP("%s\n", __func__);
+
+ /* If there are no open file references then we aren't busy */
+ scsi_lock(host);
+ if (us->open_count <= 0) {
+ us->open_count = -1; /* No more opens allowed */
+ rc = 0;
+ }
+ scsi_unlock(host);
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(usb_stor_check_busy);
+
/*
* fill_inquiry_response takes an unsigned char array (which must
* be at least 36 characters) and populates the vendor name,
@@ -1060,6 +1080,7 @@ static struct usb_driver usb_storage_dri
.reset_resume = usb_stor_reset_resume,
.pre_reset = usb_stor_pre_reset,
.post_reset = usb_stor_post_reset,
+ .check_busy = usb_stor_check_busy,
.id_table = usb_storage_usb_ids,
.supports_autosuspend = 1,
.soft_unbind = 1,
Index: usb-3.1/drivers/usb/storage/scsiglue.c
===================================================================
--- usb-3.1.orig/drivers/usb/storage/scsiglue.c
+++ usb-3.1/drivers/usb/storage/scsiglue.c
@@ -411,6 +411,32 @@ void usb_stor_report_bus_reset(struct us
scsi_unlock(host);
}

+/* Check whether to allow a device file to be opened. */
+static int device_open(struct Scsi_Host *host, struct scsi_device *sdev)
+{
+ struct us_data *us = host_to_us(host);
+ int rc = -EIO;
+
+ /* Allow the open if a disconnect hasn't been requested */
+ scsi_lock(host);
+ if (us->open_count >= 0) {
+ ++us->open_count;
+ rc = 0;
+ }
+ scsi_unlock(host);
+ return rc;
+}
+
+/* A device file has been closed. */
+static void device_close(struct Scsi_Host *host, struct scsi_device *sdev)
+{
+ struct us_data *us = host_to_us(host);
+
+ scsi_lock(host);
+ --us->open_count;
+ scsi_unlock(host);
+}
+
/***********************************************************************
* /proc/scsi/ functions
***********************************************************************/
@@ -537,6 +563,10 @@ struct scsi_host_template usb_stor_host_
.eh_device_reset_handler = device_reset,
.eh_bus_reset_handler = bus_reset,

+ /* open file reference notifiers */
+ .device_open = device_open,
+ .device_close = device_close,
+
/* queue commands only, only one command per LUN */
.can_queue = 1,
.cmd_per_lun = 1,
/* usb-try-discon -- try to claim a USB mass-storage interface */

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/ioctl.h>

#include <linux/usbdevice_fs.h>
#define USBDEVFS_TRY_DISCONNECT _IO('U', 26)


int main(int argc, char **argv)
{
const char *filename;
unsigned intf;
int fd;
int rc;
int c;
struct usbdevfs_ioctl uioctl;

if (argc != 3) {
fprintf(stderr, "Usage: usb-try-discon devpath intf-num\n");
return 1;
}
filename = argv[1];
intf = atoi(argv[2]);

fd = open(filename, O_WRONLY);
if (fd < 0) {
perror("Error opening device file");
return 1;
}

printf("Trying to disconnect driver of %s interface %d\n",
filename, intf);
uioctl.ifno = intf;
uioctl.ioctl_code = USBDEVFS_TRY_DISCONNECT;
uioctl.data = NULL;
rc = ioctl(fd, USBDEVFS_IOCTL, &uioctl);
if (rc < 0 && errno == ENODATA) {
printf("No driver was connected.\n");
} else if (rc < 0) {
perror("Error in ioctl");
return 1;
} else {
printf("Disconnect successful\n");
}

printf("Trying to claim the interface\n");
rc = ioctl(fd, USBDEVFS_CLAIMINTERFACE, &intf);
if (rc < 0) {
perror("Error in ioctl");
return 1;
}
printf("Claim successful\n");

printf("Press Enter to continue...");
do {
c = getc(stdin);
} while (c != '\n' && c != EOF);

printf("Releasing interface\n");
rc = ioctl(fd, USBDEVFS_RELEASEINTERFACE, &intf);
if (rc < 0) {
perror("Error in ioctl");
return 1;
}
printf("Release successful\n");

close(fd);
return 0;
}