From 7189b43f9280cdfa015f8f26b2c5cab89ffd9e4b Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Tue, 27 Aug 2019 12:34:36 +0200 Subject: USB: cdc-wdm: fix race between write and disconnect due to flag abuse 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 Cc: stable Link: https://lore.kernel.org/r/20190827103436.21143-1-oneukum@suse.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'drivers/usb/class/cdc-wdm.c') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 9f001659807a..217a479165e0 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -597,10 +597,20 @@ static int wdm_flush(struct file *file, fl_owner_t id) { 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); @@ -968,8 +978,6 @@ static void wdm_disconnect(struct usb_interface *intf) 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); -- cgit v1.2.3 From bdf05b2108a01ccf27124377f9329dd3793b124d Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Mon, 18 Feb 2019 22:34:51 +0800 Subject: cdc-wdm: pass return value of recover_from_urb_loss [ Upstream commit 0742a338f5b3446a26de551ad8273fb41b2787f2 ] 'rv' is the correct return value, pass it upstream instead of 0 Fixes: 17d80d562fd7 ("USB: autosuspend for cdc-wdm") Signed-off-by: YueHaibing Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin --- drivers/usb/class/cdc-wdm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/class/cdc-wdm.c') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 217a479165e0..09337a973335 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -1098,7 +1098,7 @@ static int wdm_post_reset(struct usb_interface *intf) rv = recover_from_urb_loss(desc); mutex_unlock(&desc->wlock); mutex_unlock(&desc->rlock); - return 0; + return rv; } static struct usb_driver wdm_driver = { -- cgit v1.2.3