| From 1426bd2c9f7e3126e2678e7469dca9fd9fc6dd3e Mon Sep 17 00:00:00 2001 |
| From: Oliver Neukum <oneukum@suse.com> |
| Date: Tue, 27 Aug 2019 12:34:36 +0200 |
| Subject: USB: cdc-wdm: fix race between write and disconnect due to flag abuse |
| |
| From: Oliver Neukum <oneukum@suse.com> |
| |
| commit 1426bd2c9f7e3126e2678e7469dca9fd9fc6dd3e 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: afba937e540c9 ("USB: CDC WDM driver") |
| Reported-by: syzbot+d232cca6ec42c2edb3fc@syzkaller.appspotmail.com |
| Signed-off-by: Oliver Neukum <oneukum@suse.com> |
| Cc: stable <stable@vger.kernel.org> |
| Link: https://lore.kernel.org/r/20190827103436.21143-1-oneukum@suse.com |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/usb/class/cdc-wdm.c | 16 ++++++++++++---- |
| 1 file changed, 12 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/usb/class/cdc-wdm.c |
| +++ b/drivers/usb/class/cdc-wdm.c |
| @@ -587,10 +587,20 @@ static int wdm_flush(struct file *file, |
| { |
| 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 */ |
| - 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", |
| desc->werr); |
| |
| @@ -974,8 +984,6 @@ static void wdm_disconnect(struct usb_in |
| spin_lock_irqsave(&desc->iuspin, flags); |
| set_bit(WDM_DISCONNECTING, &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); |
| wake_up_all(&desc->wait); |
| mutex_lock(&desc->rlock); |