Commit 2ca359f4 authored by Oliver Neukum's avatar Oliver Neukum Committed by Greg Kroah-Hartman

Revert "USB: rio500: simplify locking"

This reverts commit d710734b.
This simplification causes a deadlock.

Reported-by: syzbot+7bbcbe9c9ff0cd49592a@syzkaller.appspotmail.com
Fixes: d710734b ("USB: rio500: simplify locking")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: default avatarOliver Neukum <oneukum@suse.com>
Link: https://lore.kernel.org/r/20190808092854.23519-1-oneukum@suse.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent c43f28df
...@@ -51,6 +51,7 @@ struct rio_usb_data { ...@@ -51,6 +51,7 @@ struct rio_usb_data {
char *obuf, *ibuf; /* transfer buffers */ char *obuf, *ibuf; /* transfer buffers */
char bulk_in_ep, bulk_out_ep; /* Endpoint assignments */ char bulk_in_ep, bulk_out_ep; /* Endpoint assignments */
wait_queue_head_t wait_q; /* for timeouts */ wait_queue_head_t wait_q; /* for timeouts */
struct mutex lock; /* general race avoidance */
}; };
static DEFINE_MUTEX(rio500_mutex); static DEFINE_MUTEX(rio500_mutex);
...@@ -62,8 +63,10 @@ static int open_rio(struct inode *inode, struct file *file) ...@@ -62,8 +63,10 @@ static int open_rio(struct inode *inode, struct file *file)
/* against disconnect() */ /* against disconnect() */
mutex_lock(&rio500_mutex); mutex_lock(&rio500_mutex);
mutex_lock(&(rio->lock));
if (rio->isopen || !rio->present) { if (rio->isopen || !rio->present) {
mutex_unlock(&(rio->lock));
mutex_unlock(&rio500_mutex); mutex_unlock(&rio500_mutex);
return -EBUSY; return -EBUSY;
} }
...@@ -71,6 +74,7 @@ static int open_rio(struct inode *inode, struct file *file) ...@@ -71,6 +74,7 @@ static int open_rio(struct inode *inode, struct file *file)
init_waitqueue_head(&rio->wait_q); init_waitqueue_head(&rio->wait_q);
mutex_unlock(&(rio->lock));
dev_info(&rio->rio_dev->dev, "Rio opened.\n"); dev_info(&rio->rio_dev->dev, "Rio opened.\n");
mutex_unlock(&rio500_mutex); mutex_unlock(&rio500_mutex);
...@@ -84,6 +88,7 @@ static int close_rio(struct inode *inode, struct file *file) ...@@ -84,6 +88,7 @@ static int close_rio(struct inode *inode, struct file *file)
/* against disconnect() */ /* against disconnect() */
mutex_lock(&rio500_mutex); mutex_lock(&rio500_mutex);
mutex_lock(&(rio->lock));
rio->isopen = 0; rio->isopen = 0;
if (!rio->present) { if (!rio->present) {
...@@ -95,6 +100,7 @@ static int close_rio(struct inode *inode, struct file *file) ...@@ -95,6 +100,7 @@ static int close_rio(struct inode *inode, struct file *file)
} else { } else {
dev_info(&rio->rio_dev->dev, "Rio closed.\n"); dev_info(&rio->rio_dev->dev, "Rio closed.\n");
} }
mutex_unlock(&(rio->lock));
mutex_unlock(&rio500_mutex); mutex_unlock(&rio500_mutex);
return 0; return 0;
} }
...@@ -109,7 +115,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -109,7 +115,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg)
int retries; int retries;
int retval=0; int retval=0;
mutex_lock(&rio500_mutex); mutex_lock(&(rio->lock));
/* Sanity check to make sure rio is connected, powered, etc */ /* Sanity check to make sure rio is connected, powered, etc */
if (rio->present == 0 || rio->rio_dev == NULL) { if (rio->present == 0 || rio->rio_dev == NULL) {
retval = -ENODEV; retval = -ENODEV;
...@@ -253,7 +259,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -253,7 +259,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg)
err_out: err_out:
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return retval; return retval;
} }
...@@ -273,12 +279,12 @@ write_rio(struct file *file, const char __user *buffer, ...@@ -273,12 +279,12 @@ write_rio(struct file *file, const char __user *buffer,
int errn = 0; int errn = 0;
int intr; int intr;
intr = mutex_lock_interruptible(&rio500_mutex); intr = mutex_lock_interruptible(&(rio->lock));
if (intr) if (intr)
return -EINTR; return -EINTR;
/* Sanity check to make sure rio is connected, powered, etc */ /* Sanity check to make sure rio is connected, powered, etc */
if (rio->present == 0 || rio->rio_dev == NULL) { if (rio->present == 0 || rio->rio_dev == NULL) {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return -ENODEV; return -ENODEV;
} }
...@@ -301,7 +307,7 @@ write_rio(struct file *file, const char __user *buffer, ...@@ -301,7 +307,7 @@ write_rio(struct file *file, const char __user *buffer,
goto error; goto error;
} }
if (signal_pending(current)) { if (signal_pending(current)) {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return bytes_written ? bytes_written : -EINTR; return bytes_written ? bytes_written : -EINTR;
} }
...@@ -339,12 +345,12 @@ write_rio(struct file *file, const char __user *buffer, ...@@ -339,12 +345,12 @@ write_rio(struct file *file, const char __user *buffer,
buffer += copy_size; buffer += copy_size;
} while (count > 0); } while (count > 0);
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return bytes_written ? bytes_written : -EIO; return bytes_written ? bytes_written : -EIO;
error: error:
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return errn; return errn;
} }
...@@ -361,12 +367,12 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) ...@@ -361,12 +367,12 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
char *ibuf; char *ibuf;
int intr; int intr;
intr = mutex_lock_interruptible(&rio500_mutex); intr = mutex_lock_interruptible(&(rio->lock));
if (intr) if (intr)
return -EINTR; return -EINTR;
/* Sanity check to make sure rio is connected, powered, etc */ /* Sanity check to make sure rio is connected, powered, etc */
if (rio->present == 0 || rio->rio_dev == NULL) { if (rio->present == 0 || rio->rio_dev == NULL) {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return -ENODEV; return -ENODEV;
} }
...@@ -377,11 +383,11 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) ...@@ -377,11 +383,11 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
while (count > 0) { while (count > 0) {
if (signal_pending(current)) { if (signal_pending(current)) {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return read_count ? read_count : -EINTR; return read_count ? read_count : -EINTR;
} }
if (!rio->rio_dev) { if (!rio->rio_dev) {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return -ENODEV; return -ENODEV;
} }
this_read = (count >= IBUF_SIZE) ? IBUF_SIZE : count; this_read = (count >= IBUF_SIZE) ? IBUF_SIZE : count;
...@@ -399,7 +405,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) ...@@ -399,7 +405,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
count = this_read = partial; count = this_read = partial;
} else if (result == -ETIMEDOUT || result == 15) { /* FIXME: 15 ??? */ } else if (result == -ETIMEDOUT || result == 15) { /* FIXME: 15 ??? */
if (!maxretry--) { if (!maxretry--) {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
dev_err(&rio->rio_dev->dev, dev_err(&rio->rio_dev->dev,
"read_rio: maxretry timeout\n"); "read_rio: maxretry timeout\n");
return -ETIME; return -ETIME;
...@@ -409,19 +415,19 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) ...@@ -409,19 +415,19 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
finish_wait(&rio->wait_q, &wait); finish_wait(&rio->wait_q, &wait);
continue; continue;
} else if (result != -EREMOTEIO) { } else if (result != -EREMOTEIO) {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
dev_err(&rio->rio_dev->dev, dev_err(&rio->rio_dev->dev,
"Read Whoops - result:%d partial:%u this_read:%u\n", "Read Whoops - result:%d partial:%u this_read:%u\n",
result, partial, this_read); result, partial, this_read);
return -EIO; return -EIO;
} else { } else {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return (0); return (0);
} }
if (this_read) { if (this_read) {
if (copy_to_user(buffer, ibuf, this_read)) { if (copy_to_user(buffer, ibuf, this_read)) {
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return -EFAULT; return -EFAULT;
} }
count -= this_read; count -= this_read;
...@@ -429,7 +435,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) ...@@ -429,7 +435,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
buffer += this_read; buffer += this_read;
} }
} }
mutex_unlock(&rio500_mutex); mutex_unlock(&(rio->lock));
return read_count; return read_count;
} }
...@@ -494,6 +500,8 @@ static int probe_rio(struct usb_interface *intf, ...@@ -494,6 +500,8 @@ static int probe_rio(struct usb_interface *intf,
} }
dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf); dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf);
mutex_init(&(rio->lock));
usb_set_intfdata (intf, rio); usb_set_intfdata (intf, rio);
rio->present = 1; rio->present = 1;
bail_out: bail_out:
...@@ -511,10 +519,12 @@ static void disconnect_rio(struct usb_interface *intf) ...@@ -511,10 +519,12 @@ static void disconnect_rio(struct usb_interface *intf)
if (rio) { if (rio) {
usb_deregister_dev(intf, &usb_rio_class); usb_deregister_dev(intf, &usb_rio_class);
mutex_lock(&(rio->lock));
if (rio->isopen) { if (rio->isopen) {
rio->isopen = 0; rio->isopen = 0;
/* better let it finish - the release will do whats needed */ /* better let it finish - the release will do whats needed */
rio->rio_dev = NULL; rio->rio_dev = NULL;
mutex_unlock(&(rio->lock));
mutex_unlock(&rio500_mutex); mutex_unlock(&rio500_mutex);
return; return;
} }
...@@ -524,6 +534,7 @@ static void disconnect_rio(struct usb_interface *intf) ...@@ -524,6 +534,7 @@ static void disconnect_rio(struct usb_interface *intf)
dev_info(&intf->dev, "USB Rio disconnected.\n"); dev_info(&intf->dev, "USB Rio disconnected.\n");
rio->present = 0; rio->present = 0;
mutex_unlock(&(rio->lock));
} }
mutex_unlock(&rio500_mutex); mutex_unlock(&rio500_mutex);
} }
......
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