Commit 7cb7d92f authored by Oliver Neukum's avatar Oliver Neukum Committed by Kleber Sacilotto de Souza

USB: cdc-wdm: fix race between write and disconnect due to flag abuse

BugLink: https://bugs.launchpad.net/bugs/1845036

commit 1426bd2c upstream.

In case of a disconnect an ongoing flush() has to be made fail.
Nevertheless we cannot be sure that any pending URB has already
finished, so although they will never succeed, they still must
not be touched.
The clean solution for this is to check for WDM_IN_USE
and WDM_DISCONNECTED in flush(). There is no point in ever
clearing WDM_IN_USE, as no further writes make sense.

The issue is as old as the driver.

Fixes: afba937e ("USB: CDC WDM driver")
Reported-by: syzbot+d232cca6ec42c2edb3fc@syzkaller.appspotmail.com
Signed-off-by: default avatarOliver Neukum <oneukum@suse.com>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20190827103436.21143-1-oneukum@suse.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarConnor Kuehl <connor.kuehl@canonical.com>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
parent 21ee22c5
...@@ -577,10 +577,20 @@ static int wdm_flush(struct file *file, fl_owner_t id) ...@@ -577,10 +577,20 @@ static int wdm_flush(struct file *file, fl_owner_t id)
{ {
struct wdm_device *desc = file->private_data; struct wdm_device *desc = file->private_data;
wait_event(desc->wait, !test_bit(WDM_IN_USE, &desc->flags)); wait_event(desc->wait,
/*
* needs both flags. We cannot do with one
* because resetting it would cause a race
* with write() yet we need to signal
* a disconnect
*/
!test_bit(WDM_IN_USE, &desc->flags) ||
test_bit(WDM_DISCONNECTING, &desc->flags));
/* cannot dereference desc->intf if WDM_DISCONNECTING */ /* cannot dereference desc->intf if WDM_DISCONNECTING */
if (desc->werr < 0 && !test_bit(WDM_DISCONNECTING, &desc->flags)) if (test_bit(WDM_DISCONNECTING, &desc->flags))
return -ENODEV;
if (desc->werr < 0)
dev_err(&desc->intf->dev, "Error in flush path: %d\n", dev_err(&desc->intf->dev, "Error in flush path: %d\n",
desc->werr); desc->werr);
...@@ -968,8 +978,6 @@ static void wdm_disconnect(struct usb_interface *intf) ...@@ -968,8 +978,6 @@ static void wdm_disconnect(struct usb_interface *intf)
spin_lock_irqsave(&desc->iuspin, flags); spin_lock_irqsave(&desc->iuspin, flags);
set_bit(WDM_DISCONNECTING, &desc->flags); set_bit(WDM_DISCONNECTING, &desc->flags);
set_bit(WDM_READ, &desc->flags); set_bit(WDM_READ, &desc->flags);
/* to terminate pending flushes */
clear_bit(WDM_IN_USE, &desc->flags);
spin_unlock_irqrestore(&desc->iuspin, flags); spin_unlock_irqrestore(&desc->iuspin, flags);
wake_up_all(&desc->wait); wake_up_all(&desc->wait);
mutex_lock(&desc->rlock); mutex_lock(&desc->rlock);
......
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