lkml.org 
[lkml]   [2019]   [Aug]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.2 017/144] Revert "USB: rio500: simplify locking"
    Date
    From: Oliver Neukum <oneukum@suse.com>

    commit 2ca359f4f8b954b3a9d15a89f22a8b7283e7669f upstream.

    This reverts commit d710734b06770814de2bfa2819420fb5df7f3a81.
    This simplification causes a deadlock.

    Reported-by: syzbot+7bbcbe9c9ff0cd49592a@syzkaller.appspotmail.com
    Fixes: d710734b0677 ("USB: rio500: simplify locking")
    Cc: stable <stable@vger.kernel.org>
    Signed-off-by: Oliver Neukum <oneukum@suse.com>
    Link: https://lore.kernel.org/r/20190808092854.23519-1-oneukum@suse.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/usb/misc/rio500.c | 43 +++++++++++++++++++++++++++----------------
    1 file changed, 27 insertions(+), 16 deletions(-)

    --- a/drivers/usb/misc/rio500.c
    +++ b/drivers/usb/misc/rio500.c
    @@ -51,6 +51,7 @@ struct rio_usb_data {
    char *obuf, *ibuf; /* transfer buffers */
    char bulk_in_ep, bulk_out_ep; /* Endpoint assignments */
    wait_queue_head_t wait_q; /* for timeouts */
    + struct mutex lock; /* general race avoidance */
    };

    static DEFINE_MUTEX(rio500_mutex);
    @@ -62,8 +63,10 @@ static int open_rio(struct inode *inode,

    /* against disconnect() */
    mutex_lock(&rio500_mutex);
    + mutex_lock(&(rio->lock));

    if (rio->isopen || !rio->present) {
    + mutex_unlock(&(rio->lock));
    mutex_unlock(&rio500_mutex);
    return -EBUSY;
    }
    @@ -71,6 +74,7 @@ static int open_rio(struct inode *inode,

    init_waitqueue_head(&rio->wait_q);

    + mutex_unlock(&(rio->lock));

    dev_info(&rio->rio_dev->dev, "Rio opened.\n");
    mutex_unlock(&rio500_mutex);
    @@ -84,6 +88,7 @@ static int close_rio(struct inode *inode

    /* against disconnect() */
    mutex_lock(&rio500_mutex);
    + mutex_lock(&(rio->lock));

    rio->isopen = 0;
    if (!rio->present) {
    @@ -95,6 +100,7 @@ static int close_rio(struct inode *inode
    } else {
    dev_info(&rio->rio_dev->dev, "Rio closed.\n");
    }
    + mutex_unlock(&(rio->lock));
    mutex_unlock(&rio500_mutex);
    return 0;
    }
    @@ -109,7 +115,7 @@ static long ioctl_rio(struct file *file,
    int retries;
    int retval=0;

    - mutex_lock(&rio500_mutex);
    + mutex_lock(&(rio->lock));
    /* Sanity check to make sure rio is connected, powered, etc */
    if (rio->present == 0 || rio->rio_dev == NULL) {
    retval = -ENODEV;
    @@ -253,7 +259,7 @@ static long ioctl_rio(struct file *file,


    err_out:
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return retval;
    }

    @@ -273,12 +279,12 @@ write_rio(struct file *file, const char
    int errn = 0;
    int intr;

    - intr = mutex_lock_interruptible(&rio500_mutex);
    + intr = mutex_lock_interruptible(&(rio->lock));
    if (intr)
    return -EINTR;
    /* Sanity check to make sure rio is connected, powered, etc */
    if (rio->present == 0 || rio->rio_dev == NULL) {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return -ENODEV;
    }

    @@ -301,7 +307,7 @@ write_rio(struct file *file, const char
    goto error;
    }
    if (signal_pending(current)) {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return bytes_written ? bytes_written : -EINTR;
    }

    @@ -339,12 +345,12 @@ write_rio(struct file *file, const char
    buffer += copy_size;
    } while (count > 0);

    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));

    return bytes_written ? bytes_written : -EIO;

    error:
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return errn;
    }

    @@ -361,12 +367,12 @@ read_rio(struct file *file, char __user
    char *ibuf;
    int intr;

    - intr = mutex_lock_interruptible(&rio500_mutex);
    + intr = mutex_lock_interruptible(&(rio->lock));
    if (intr)
    return -EINTR;
    /* Sanity check to make sure rio is connected, powered, etc */
    if (rio->present == 0 || rio->rio_dev == NULL) {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return -ENODEV;
    }

    @@ -377,11 +383,11 @@ read_rio(struct file *file, char __user

    while (count > 0) {
    if (signal_pending(current)) {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return read_count ? read_count : -EINTR;
    }
    if (!rio->rio_dev) {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return -ENODEV;
    }
    this_read = (count >= IBUF_SIZE) ? IBUF_SIZE : count;
    @@ -399,7 +405,7 @@ read_rio(struct file *file, char __user
    count = this_read = partial;
    } else if (result == -ETIMEDOUT || result == 15) { /* FIXME: 15 ??? */
    if (!maxretry--) {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    dev_err(&rio->rio_dev->dev,
    "read_rio: maxretry timeout\n");
    return -ETIME;
    @@ -409,19 +415,19 @@ read_rio(struct file *file, char __user
    finish_wait(&rio->wait_q, &wait);
    continue;
    } else if (result != -EREMOTEIO) {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    dev_err(&rio->rio_dev->dev,
    "Read Whoops - result:%d partial:%u this_read:%u\n",
    result, partial, this_read);
    return -EIO;
    } else {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return (0);
    }

    if (this_read) {
    if (copy_to_user(buffer, ibuf, this_read)) {
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return -EFAULT;
    }
    count -= this_read;
    @@ -429,7 +435,7 @@ read_rio(struct file *file, char __user
    buffer += this_read;
    }
    }
    - mutex_unlock(&rio500_mutex);
    + mutex_unlock(&(rio->lock));
    return read_count;
    }

    @@ -494,6 +500,8 @@ static int probe_rio(struct usb_interfac
    }
    dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf);

    + mutex_init(&(rio->lock));
    +
    usb_set_intfdata (intf, rio);
    rio->present = 1;
    bail_out:
    @@ -511,10 +519,12 @@ static void disconnect_rio(struct usb_in
    if (rio) {
    usb_deregister_dev(intf, &usb_rio_class);

    + mutex_lock(&(rio->lock));
    if (rio->isopen) {
    rio->isopen = 0;
    /* better let it finish - the release will do whats needed */
    rio->rio_dev = NULL;
    + mutex_unlock(&(rio->lock));
    mutex_unlock(&rio500_mutex);
    return;
    }
    @@ -524,6 +534,7 @@ static void disconnect_rio(struct usb_in
    dev_info(&intf->dev, "USB Rio disconnected.\n");

    rio->present = 0;
    + mutex_unlock(&(rio->lock));
    }
    mutex_unlock(&rio500_mutex);
    }

    \
     
     \ /
      Last update: 2019-08-14 19:03    [W:5.608 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site