USB: serial: digi_acceleport: fix write-wakeup deadlocks
authorJohan Hovold <johan@kernel.org>
Mon, 26 Oct 2020 10:43:06 +0000 (11:43 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 30 Dec 2020 10:54:12 +0000 (11:54 +0100)
commit 5098e77962e7c8947f87bd8c5869c83e000a522a upstream.

The driver must not call tty_wakeup() while holding its private lock as
line disciplines are allowed to call back into write() from
write_wakeup(), leading to a deadlock.

Also remove the unneeded work struct that was used to defer wakeup in
order to work around a possible race in ancient times (see comment about
n_tty write_chan() in commit 14b54e39b412 ("USB: serial: remove
changelogs and old todo entries")).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/serial/digi_acceleport.c

index 91055a1..0d606fa 100644 (file)
@@ -19,7 +19,6 @@
 #include <linux/tty_flip.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
-#include <linux/workqueue.h>
 #include <linux/uaccess.h>
 #include <linux/usb.h>
 #include <linux/wait.h>
@@ -198,14 +197,12 @@ struct digi_port {
        int dp_throttle_restart;
        wait_queue_head_t dp_flush_wait;
        wait_queue_head_t dp_close_wait;        /* wait queue for close */
-       struct work_struct dp_wakeup_work;
        struct usb_serial_port *dp_port;
 };
 
 
 /* Local Function Declarations */
 
-static void digi_wakeup_write_lock(struct work_struct *work);
 static int digi_write_oob_command(struct usb_serial_port *port,
        unsigned char *buf, int count, int interruptible);
 static int digi_write_inb_command(struct usb_serial_port *port,
@@ -356,26 +353,6 @@ __releases(lock)
        return timeout;
 }
 
-
-/*
- *  Digi Wakeup Write
- *
- *  Wake up port, line discipline, and tty processes sleeping
- *  on writes.
- */
-
-static void digi_wakeup_write_lock(struct work_struct *work)
-{
-       struct digi_port *priv =
-                       container_of(work, struct digi_port, dp_wakeup_work);
-       struct usb_serial_port *port = priv->dp_port;
-       unsigned long flags;
-
-       spin_lock_irqsave(&priv->dp_port_lock, flags);
-       tty_port_tty_wakeup(&port->port);
-       spin_unlock_irqrestore(&priv->dp_port_lock, flags);
-}
-
 /*
  *  Digi Write OOB Command
  *
@@ -986,6 +963,7 @@ static void digi_write_bulk_callback(struct urb *urb)
        unsigned long flags;
        int ret = 0;
        int status = urb->status;
+       bool wakeup;
 
        /* port and serial sanity check */
        if (port == NULL || (priv = usb_get_serial_port_data(port)) == NULL) {
@@ -1012,6 +990,7 @@ static void digi_write_bulk_callback(struct urb *urb)
        }
 
        /* try to send any buffered data on this port */
+       wakeup = true;
        spin_lock_irqsave(&priv->dp_port_lock, flags);
        priv->dp_write_urb_in_use = 0;
        if (priv->dp_out_buf_len > 0) {
@@ -1027,19 +1006,18 @@ static void digi_write_bulk_callback(struct urb *urb)
                if (ret == 0) {
                        priv->dp_write_urb_in_use = 1;
                        priv->dp_out_buf_len = 0;
+                       wakeup = false;
                }
        }
-       /* wake up processes sleeping on writes immediately */
-       tty_port_tty_wakeup(&port->port);
-       /* also queue up a wakeup at scheduler time, in case we */
-       /* lost the race in write_chan(). */
-       schedule_work(&priv->dp_wakeup_work);
-
        spin_unlock_irqrestore(&priv->dp_port_lock, flags);
+
        if (ret && ret != -EPERM)
                dev_err_console(port,
                        "%s: usb_submit_urb failed, ret=%d, port=%d\n",
                        __func__, ret, priv->dp_port_num);
+
+       if (wakeup)
+               tty_port_tty_wakeup(&port->port);
 }
 
 static int digi_write_room(struct tty_struct *tty)
@@ -1239,7 +1217,6 @@ static int digi_port_init(struct usb_serial_port *port, unsigned port_num)
        init_waitqueue_head(&priv->dp_transmit_idle_wait);
        init_waitqueue_head(&priv->dp_flush_wait);
        init_waitqueue_head(&priv->dp_close_wait);
-       INIT_WORK(&priv->dp_wakeup_work, digi_wakeup_write_lock);
        priv->dp_port = port;
 
        init_waitqueue_head(&port->write_wait);
@@ -1508,13 +1485,14 @@ static int digi_read_oob_callback(struct urb *urb)
                        rts = C_CRTSCTS(tty);
 
                if (tty && opcode == DIGI_CMD_READ_INPUT_SIGNALS) {
+                       bool wakeup = false;
+
                        spin_lock_irqsave(&priv->dp_port_lock, flags);
                        /* convert from digi flags to termiox flags */
                        if (val & DIGI_READ_INPUT_SIGNALS_CTS) {
                                priv->dp_modem_signals |= TIOCM_CTS;
-                               /* port must be open to use tty struct */
                                if (rts)
-                                       tty_port_tty_wakeup(&port->port);
+                                       wakeup = true;
                        } else {
                                priv->dp_modem_signals &= ~TIOCM_CTS;
                                /* port must be open to use tty struct */
@@ -1533,6 +1511,9 @@ static int digi_read_oob_callback(struct urb *urb)
                                priv->dp_modem_signals &= ~TIOCM_CD;
 
                        spin_unlock_irqrestore(&priv->dp_port_lock, flags);
+
+                       if (wakeup)
+                               tty_port_tty_wakeup(&port->port);
                } else if (opcode == DIGI_CMD_TRANSMIT_IDLE) {
                        spin_lock_irqsave(&priv->dp_port_lock, flags);
                        priv->dp_transmit_idle = 1;