From 27cd2e25b37af973b61b77217fa2dad822889ff8 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Wed, 24 Jun 2020 10:52:03 +0200 Subject: [PATCH] CDC-WDM: fix hangs in flush() When flushing a task needs to wait a bounded time, as a hardware failure could mean eternal sleep. So an arbitrary timeout is introduced. Simply making the syscall interruptible will not do the job, as while the syscall would not hang, the fd would be unclosable. In addition a flush() and a write() may be waiting for the same IO to complete. Hence completion of output must use wake_up_all(), even in error handling. Reported-by: Tetsuo Handa Signed-off-by: Oliver Neukum --- drivers/usb/class/cdc-wdm.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index e3db6fbeadef..ec5412773c57 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -58,6 +58,9 @@ MODULE_DEVICE_TABLE (usb, wdm_ids); #define WDM_MAX 16 +/* flush() needs to be uninterruptible, but we cannot wait forever */ +#define WDM_FLUSH_TIMEOUT (30 * HZ) + /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)" */ #define WDM_DEFAULT_BUFSIZE 256 @@ -151,7 +154,7 @@ static void wdm_out_callback(struct urb *urb) kfree(desc->outbuf); desc->outbuf = NULL; clear_bit(WDM_IN_USE, &desc->flags); - wake_up(&desc->wait); + wake_up_all(&desc->wait); } static void wdm_in_callback(struct urb *urb) @@ -424,6 +427,7 @@ static ssize_t wdm_write if (rv < 0) { desc->outbuf = NULL; clear_bit(WDM_IN_USE, &desc->flags); + wake_up_all(&desc->wait); /* for flush() */ dev_err(&desc->intf->dev, "Tx URB error: %d\n", rv); rv = usb_translate_errors(rv); goto out_free_mem_pm; @@ -586,8 +590,9 @@ static ssize_t wdm_read static int wdm_flush(struct file *file, fl_owner_t id) { struct wdm_device *desc = file->private_data; + int rv; - wait_event(desc->wait, + rv = wait_event_interruptible_timeout(desc->wait, /* * needs both flags. We cannot do with one * because resetting it would cause a race @@ -595,11 +600,16 @@ static int wdm_flush(struct file *file, fl_owner_t id) * a disconnect */ !test_bit(WDM_IN_USE, &desc->flags) || - test_bit(WDM_DISCONNECTING, &desc->flags)); + test_bit(WDM_DISCONNECTING, &desc->flags), + WDM_FLUSH_TIMEOUT); /* cannot dereference desc->intf if WDM_DISCONNECTING */ if (test_bit(WDM_DISCONNECTING, &desc->flags)) return -ENODEV; + if (!rv) + return -EIO; + if (rv < 0) + return -EINTR; if (desc->werr < 0) dev_err(&desc->intf->dev, "Error in flush path: %d\n", desc->werr); @@ -656,6 +666,14 @@ static int wdm_open(struct inode *inode, struct file *file) goto out; } + /* + * in case flush() had timed out + */ + usb_kill_urb(desc->command); + spin_lock_irq(&desc->iuspin); + desc->werr = 0; + spin_unlock_irq(&desc->iuspin); + /* using write lock to protect desc->count */ mutex_lock(&desc->wlock); if (!desc->count++) { -- 2.16.4