Commit 8e082a0d authored by Duncan Sands's avatar Duncan Sands Committed by Greg Kroah-Hartman

[PATCH] USB usbfs: take a reference to the usb device

Hi Greg, this is the first of a series of patches that replace the
per-file semaphore ps->devsem with the per-device semaphore
ps->dev->serialize.  The role of devsem was to protect against
device disconnection.  This can be done equally well using
ps->dev->serialize.  On the other hand, ps->dev->serialize
protects against configuration and other changes, and has
already been introduced into usbfs in several places.  Using
just one semaphore simplifies the code and removes some
remaining race conditions.  It should also fix the oopses some
people have been seeing.  In this first patch, a reference is
taken to the usb device as long as the usbfs file is open.  That
way we can use ps->dev->serialize for as long as ps exists.

 devio.c |   27 ++++++++++++++++-----------
 inode.c |    3 ---
 2 files changed, 16 insertions(+), 14 deletions(-)
parent 75f00f32
...@@ -60,6 +60,11 @@ struct async { ...@@ -60,6 +60,11 @@ struct async {
struct urb *urb; struct urb *urb;
}; };
static inline int connected (struct usb_device *dev)
{
return dev->state != USB_STATE_NOTATTACHED;
}
static loff_t usbdev_lseek(struct file *file, loff_t offset, int orig) static loff_t usbdev_lseek(struct file *file, loff_t offset, int orig)
{ {
loff_t ret; loff_t ret;
...@@ -94,7 +99,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l ...@@ -94,7 +99,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l
pos = *ppos; pos = *ppos;
down_read(&ps->devsem); down_read(&ps->devsem);
if (!ps->dev) { if (!connected(ps->dev)) {
ret = -ENODEV; ret = -ENODEV;
goto err; goto err;
} else if (pos < 0) { } else if (pos < 0) {
...@@ -343,8 +348,6 @@ static void driver_disconnect(struct usb_interface *intf) ...@@ -343,8 +348,6 @@ static void driver_disconnect(struct usb_interface *intf)
* all pending I/O requests; 2.6 does that. * all pending I/O requests; 2.6 does that.
*/ */
/* prevent new I/O requests */
ps->dev = 0;
clear_bit(intf->cur_altsetting->desc.bInterfaceNumber, &ps->ifclaimed); clear_bit(intf->cur_altsetting->desc.bInterfaceNumber, &ps->ifclaimed);
usb_set_intfdata (intf, NULL); usb_set_intfdata (intf, NULL);
...@@ -365,7 +368,7 @@ static int claimintf(struct dev_state *ps, unsigned int intf) ...@@ -365,7 +368,7 @@ static int claimintf(struct dev_state *ps, unsigned int intf)
struct usb_interface *iface; struct usb_interface *iface;
int err; int err;
if (intf >= 8*sizeof(ps->ifclaimed) || !dev if (intf >= 8*sizeof(ps->ifclaimed)
|| intf >= dev->actconfig->desc.bNumInterfaces) || intf >= dev->actconfig->desc.bNumInterfaces)
return -EINVAL; return -EINVAL;
/* already claimed */ /* already claimed */
...@@ -506,7 +509,7 @@ static int usbdev_open(struct inode *inode, struct file *file) ...@@ -506,7 +509,7 @@ static int usbdev_open(struct inode *inode, struct file *file)
lock_kernel(); lock_kernel();
ret = -ENOENT; ret = -ENOENT;
dev = inode->u.generic_ip; dev = usb_get_dev(inode->u.generic_ip);
if (!dev) { if (!dev) {
kfree(ps); kfree(ps);
goto out; goto out;
...@@ -540,13 +543,15 @@ static int usbdev_release(struct inode *inode, struct file *file) ...@@ -540,13 +543,15 @@ static int usbdev_release(struct inode *inode, struct file *file)
lock_kernel(); lock_kernel();
list_del_init(&ps->list); list_del_init(&ps->list);
if (ps->dev) { if (connected(ps->dev)) {
for (i = 0; ps->ifclaimed && i < 8*sizeof(ps->ifclaimed); i++) for (i = 0; ps->ifclaimed && i < 8*sizeof(ps->ifclaimed); i++)
if (test_bit(i, &ps->ifclaimed)) if (test_bit(i, &ps->ifclaimed))
releaseintf(ps, i); releaseintf(ps, i);
destroy_all_async(ps);
} }
unlock_kernel(); unlock_kernel();
destroy_all_async(ps); usb_put_dev(ps->dev);
ps->dev = NULL;
kfree(ps); kfree(ps);
return 0; return 0;
} }
...@@ -1021,7 +1026,7 @@ static int proc_reapurb(struct dev_state *ps, void __user *arg) ...@@ -1021,7 +1026,7 @@ static int proc_reapurb(struct dev_state *ps, void __user *arg)
int ret; int ret;
add_wait_queue(&ps->wait, &wait); add_wait_queue(&ps->wait, &wait);
while (ps->dev) { while (connected(ps->dev)) {
__set_current_state(TASK_INTERRUPTIBLE); __set_current_state(TASK_INTERRUPTIBLE);
if ((as = async_getcompleted(ps))) if ((as = async_getcompleted(ps)))
break; break;
...@@ -1131,7 +1136,7 @@ static int proc_ioctl (struct dev_state *ps, void __user *arg) ...@@ -1131,7 +1136,7 @@ static int proc_ioctl (struct dev_state *ps, void __user *arg)
} }
} }
if (!ps->dev) { if (!connected(ps->dev)) {
if (buf) if (buf)
kfree(buf); kfree(buf);
return -ENODEV; return -ENODEV;
...@@ -1201,7 +1206,7 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd ...@@ -1201,7 +1206,7 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd
if (!(file->f_mode & FMODE_WRITE)) if (!(file->f_mode & FMODE_WRITE))
return -EPERM; return -EPERM;
down_read(&ps->devsem); down_read(&ps->devsem);
if (!ps->dev) { if (!connected(ps->dev)) {
up_read(&ps->devsem); up_read(&ps->devsem);
return -ENODEV; return -ENODEV;
} }
...@@ -1299,7 +1304,7 @@ static unsigned int usbdev_poll(struct file *file, struct poll_table_struct *wai ...@@ -1299,7 +1304,7 @@ static unsigned int usbdev_poll(struct file *file, struct poll_table_struct *wai
poll_wait(file, &ps->wait, wait); poll_wait(file, &ps->wait, wait);
if (file->f_mode & FMODE_WRITE && !list_empty(&ps->async_completed)) if (file->f_mode & FMODE_WRITE && !list_empty(&ps->async_completed))
mask |= POLLOUT | POLLWRNORM; mask |= POLLOUT | POLLWRNORM;
if (!ps->dev) if (!connected(ps->dev))
mask |= POLLERR | POLLHUP; mask |= POLLERR | POLLHUP;
return mask; return mask;
} }
......
...@@ -717,9 +717,6 @@ void usbfs_remove_device(struct usb_device *dev) ...@@ -717,9 +717,6 @@ void usbfs_remove_device(struct usb_device *dev)
while (!list_empty(&dev->filelist)) { while (!list_empty(&dev->filelist)) {
ds = list_entry(dev->filelist.next, struct dev_state, list); ds = list_entry(dev->filelist.next, struct dev_state, list);
list_del_init(&ds->list); list_del_init(&ds->list);
down_write(&ds->devsem);
ds->dev = NULL;
up_write(&ds->devsem);
if (ds->discsignr) { if (ds->discsignr) {
sinfo.si_signo = SIGPIPE; sinfo.si_signo = SIGPIPE;
sinfo.si_errno = EPIPE; sinfo.si_errno = EPIPE;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment