From 5b06470816fb5e658e81db2a55b530ff2ba711c9 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Thu, 8 Feb 2007 15:42:53 +0100 Subject: USB: fix autosuspend race in skeleton driver as the skeleton driver was made ready for autosuspend a race condition was introduced. The reference to get device must be gotten before the autosuspend counter is upped, as this operation may sleep, dropping BKL. Dropping BKL means that the pointer to the device may become invalid. Here's the fix. Signed-off-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/usb-skeleton.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers/usb/usb-skeleton.c') diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c index 296b091cf168..46929a1b6f24 100644 --- a/drivers/usb/usb-skeleton.c +++ b/drivers/usb/usb-skeleton.c @@ -90,13 +90,15 @@ static int skel_open(struct inode *inode, struct file *file) goto exit; } + /* increment our usage count for the device */ + kref_get(&dev->kref); + /* prevent the device from being autosuspended */ retval = usb_autopm_get_interface(interface); - if (retval) + if (retval) { + kref_put(&dev->kref, skel_delete); goto exit; - - /* increment our usage count for the device */ - kref_get(&dev->kref); + } /* save our object in the file's private structure */ file->private_data = dev; -- cgit v1.2.3 From ba35e02bdcbd3a25238421a7e20efdb69436d3cf Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Thu, 1 Mar 2007 14:31:02 +0100 Subject: USB: fix skeleton driver compilation of the skeleton driver is currently broken. It doesn't compile. So while I am it: - fix typo - add comments to answer common questions - actually allow autosuspend in the driver struct - increase paralellism by restricting code under locks Signed-off-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/usb-skeleton.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'drivers/usb/usb-skeleton.c') diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c index 46929a1b6f24..962b28cd3a79 100644 --- a/drivers/usb/usb-skeleton.c +++ b/drivers/usb/usb-skeleton.c @@ -40,12 +40,16 @@ MODULE_DEVICE_TABLE(usb, skel_table); /* our private defines. if this grows any larger, use your own .h file */ #define MAX_TRANSFER (PAGE_SIZE - 512) +/* MAX_TRANSFER is chosen so that the VM is not stressed by + allocations > PAGE_SIZE and the number of packets in a page + is an integer 512 is the largest possible packet on EHCI */ #define WRITES_IN_FLIGHT 8 +/* arbitrarily chosen */ /* Structure to hold all of our device specific stuff */ struct usb_skel { - struct usb_device *dev; /* the usb device for this device */ - struct usb_interface *interface; /* the interface for this device */ + struct usb_device *udev; /* the usb device for this device */ + struct usb_interface *interface; /* the interface for this device */ struct semaphore limit_sem; /* limiting the number of writes in progress */ unsigned char *bulk_in_buffer; /* the buffer to receive data */ size_t bulk_in_size; /* the size of the receive buffer */ @@ -201,12 +205,6 @@ static ssize_t skel_write(struct file *file, const char *user_buffer, size_t cou goto exit; } - mutex_lock(&dev->io_mutex); - if (!dev->interface) { /* disconnect() was called */ - retval = -ENODEV; - goto error; - } - /* create a urb, and a buffer for it, and copy the data to the urb */ urb = usb_alloc_urb(0, GFP_KERNEL); if (!urb) { @@ -225,6 +223,14 @@ static ssize_t skel_write(struct file *file, const char *user_buffer, size_t cou goto error; } + /* this lock makes sure we don't submit URBs to gone devices */ + mutex_lock(&dev->io_mutex); + if (!dev->interface) { /* disconnect() was called */ + mutex_unlock(&dev->io_mutex); + retval = -ENODEV; + goto error; + } + /* initialize the urb properly */ usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, dev->bulk_out_endpointAddr), @@ -233,6 +239,7 @@ static ssize_t skel_write(struct file *file, const char *user_buffer, size_t cou /* send the data out the bulk port */ retval = usb_submit_urb(urb, GFP_KERNEL); + mutex_unlock(&dev->io_mutex); if (retval) { err("%s - failed submitting write urb, error %d", __FUNCTION__, retval); goto error; @@ -241,7 +248,7 @@ static ssize_t skel_write(struct file *file, const char *user_buffer, size_t cou /* release our reference to this urb, the USB core will eventually free it entirely */ usb_free_urb(urb); - mutex_unlock(&dev->io_mutex); + return writesize; error: @@ -249,7 +256,6 @@ error: usb_buffer_free(dev->udev, writesize, buf, urb->transfer_dma); usb_free_urb(urb); } - mutex_unlock(&dev->io_mutex); up(&dev->limit_sem); exit: @@ -344,6 +350,7 @@ static int skel_probe(struct usb_interface *interface, const struct usb_device_i error: if (dev) + /* this frees allocated memory */ kref_put(&dev->kref, skel_delete); return retval; } @@ -361,13 +368,14 @@ static void skel_disconnect(struct usb_interface *interface) /* give back our minor */ usb_deregister_dev(interface, &skel_class); + unlock_kernel(); /* prevent more I/O from starting */ mutex_lock(&dev->io_mutex); dev->interface = NULL; mutex_unlock(&dev->io_mutex); - unlock_kernel(); + /* decrement our usage count */ kref_put(&dev->kref, skel_delete); @@ -380,6 +388,7 @@ static struct usb_driver skel_driver = { .probe = skel_probe, .disconnect = skel_disconnect, .id_table = skel_table, + .supports_autosuspend = 1, }; static int __init usb_skel_init(void) -- cgit v1.2.3 From 5d9b89b33f3ed19479dc5240986b0fedda08b82c Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Thu, 1 Mar 2007 23:07:32 +0100 Subject: USB: kill BKL in skeleton driver Iet's kill BKL where we can. This is relative to the last patch to the skeleton driver. Signed-off-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/usb-skeleton.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'drivers/usb/usb-skeleton.c') diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c index 962b28cd3a79..8432bf171d2e 100644 --- a/drivers/usb/usb-skeleton.c +++ b/drivers/usb/usb-skeleton.c @@ -34,6 +34,9 @@ static struct usb_device_id skel_table [] = { }; MODULE_DEVICE_TABLE(usb, skel_table); +/* to prevent a race between open and disconnect */ +static DEFINE_MUTEX(skel_open_lock); + /* Get a minor range for your devices from the usb maintainer */ #define USB_SKEL_MINOR_BASE 192 @@ -80,8 +83,10 @@ static int skel_open(struct inode *inode, struct file *file) subminor = iminor(inode); + mutex_lock(&skel_open_lock); interface = usb_find_interface(&skel_driver, subminor); if (!interface) { + mutex_unlock(&skel_open_lock); err ("%s - error, can't find device for minor %d", __FUNCTION__, subminor); retval = -ENODEV; @@ -90,12 +95,15 @@ static int skel_open(struct inode *inode, struct file *file) dev = usb_get_intfdata(interface); if (!dev) { + mutex_unlock(&skel_open_lock); retval = -ENODEV; goto exit; } /* increment our usage count for the device */ kref_get(&dev->kref); + /* now we can drop the lock */ + mutex_unlock(&skel_open_lock); /* prevent the device from being autosuspended */ retval = usb_autopm_get_interface(interface); @@ -361,14 +369,14 @@ static void skel_disconnect(struct usb_interface *interface) int minor = interface->minor; /* prevent skel_open() from racing skel_disconnect() */ - lock_kernel(); + mutex_lock(&skel_open_lock); dev = usb_get_intfdata(interface); usb_set_intfdata(interface, NULL); /* give back our minor */ usb_deregister_dev(interface, &skel_class); - unlock_kernel(); + mutex_unlock(&skel_open_lock); /* prevent more I/O from starting */ mutex_lock(&dev->io_mutex); -- cgit v1.2.3