Skip to content

Commit

Permalink
cdc-wdm: Fix more races on the read path
Browse files Browse the repository at this point in the history
We must not allow the input buffer length to change while we're
shuffling the buffer contents.  We also mustn't clear the WDM_READ
flag after more data might have arrived.  Therefore move both of these
into the spinlocked region at the bottom of wdm_read().

When reading desc->length without holding the iuspin lock, use
ACCESS_ONCE() to ensure the compiler doesn't re-read it with
inconsistent results.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Tested-by: Bjørn Mork <bjorn@mork.no>
Cc: Oliver Neukum <oliver@neukum.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Ben Hutchings authored and Greg Kroah-Hartman committed Feb 24, 2012
1 parent c192c8e commit 711c68b
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions drivers/usb/class/cdc-wdm.c
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ static ssize_t wdm_write
static ssize_t wdm_read
(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
{
int rv, cntr = 0;
int rv, cntr;
int i = 0;
struct wdm_device *desc = file->private_data;

Expand All @@ -400,7 +400,8 @@ static ssize_t wdm_read
if (rv < 0)
return -ERESTARTSYS;

if (desc->length == 0) {
cntr = ACCESS_ONCE(desc->length);
if (cntr == 0) {
desc->read = 0;
retry:
if (test_bit(WDM_DISCONNECTING, &desc->flags)) {
Expand Down Expand Up @@ -455,25 +456,30 @@ static ssize_t wdm_read
goto retry;
}
clear_bit(WDM_READ, &desc->flags);
cntr = desc->length;
spin_unlock_irq(&desc->iuspin);
}

cntr = count > desc->length ? desc->length : count;
if (cntr > count)
cntr = count;
rv = copy_to_user(buffer, desc->ubuf, cntr);
if (rv > 0) {
rv = -EFAULT;
goto err;
}

spin_lock_irq(&desc->iuspin);

for (i = 0; i < desc->length - cntr; i++)
desc->ubuf[i] = desc->ubuf[i + cntr];

spin_lock_irq(&desc->iuspin);
desc->length -= cntr;
spin_unlock_irq(&desc->iuspin);
/* in case we had outstanding data */
if (!desc->length)
clear_bit(WDM_READ, &desc->flags);

spin_unlock_irq(&desc->iuspin);

rv = cntr;

err:
Expand Down

0 comments on commit 711c68b

Please sign in to comment.