cdc-wdm: Fix more races on the read path (711c68b3) · Commits · 戴 / test

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>

drivers/usb/class/cdc-wdm.c

+11 −5

Original line number	Diff line number	Diff line
		@@ -391,7 +391,7 @@ outnl:
		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;

		@@ -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)) {
		@@ -455,25 +456,30 @@ retry:
		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:

Admin message