From 4e2da44691cffbfffb1535f478d19bc2dca3e62b Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Jan 2017 19:15:10 +0100 Subject: [PATCH 01/12] USB: serial: ch341: fix initial modem-control state DTR and RTS will be asserted by the tty-layer when the port is opened and deasserted on close (if HUPCL is set). Make sure the initial state is not-asserted before the port is first opened as well. Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 2597b83a8ae2..d133e72fe888 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -258,7 +258,6 @@ static int ch341_port_probe(struct usb_serial_port *port) spin_lock_init(&priv->lock); priv->baud_rate = DEFAULT_BAUD_RATE; - priv->line_control = CH341_BIT_RTS | CH341_BIT_DTR; r = ch341_configure(port->serial->dev, priv); if (r < 0) From a20047f36e2f6a1eea4f1fd261aaa55882369868 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Jan 2017 19:15:11 +0100 Subject: [PATCH 02/12] USB: serial: ch341: fix open and resume after B0 The private baud_rate variable is used to configure the port at open and reset-resume and must never be set to (and left at) zero or reset-resume and all further open attempts will fail. Fixes: aa91def41a7b ("USB: ch341: set tty baud speed according to tty struct") Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index d133e72fe888..6279df905c14 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -355,7 +355,6 @@ static void ch341_set_termios(struct tty_struct *tty, baud_rate = tty_get_baud_rate(tty); - priv->baud_rate = baud_rate; ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX; switch (C_CSIZE(tty)) { @@ -388,6 +387,9 @@ static void ch341_set_termios(struct tty_struct *tty, spin_lock_irqsave(&priv->lock, flags); priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); spin_unlock_irqrestore(&priv->lock, flags); + + priv->baud_rate = baud_rate; + r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl); if (r < 0 && old_termios) { priv->baud_rate = tty_termios_baud_rate(old_termios); From 030ee7ae52a46a2be52ccc8242c4a330aba8d38e Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Jan 2017 19:15:12 +0100 Subject: [PATCH 03/12] USB: serial: ch341: fix modem-control and B0 handling The modem-control signals are managed by the tty-layer during open and should not be asserted prematurely when set_termios is called from driver open. Also make sure that the signals are asserted only when changing speed from B0. Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 6279df905c14..0cc5056b304d 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -384,10 +384,6 @@ static void ch341_set_termios(struct tty_struct *tty, ctrl |= CH341_LCR_STOP_BITS_2; if (baud_rate) { - spin_lock_irqsave(&priv->lock, flags); - priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); - spin_unlock_irqrestore(&priv->lock, flags); - priv->baud_rate = baud_rate; r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl); @@ -395,14 +391,16 @@ static void ch341_set_termios(struct tty_struct *tty, priv->baud_rate = tty_termios_baud_rate(old_termios); tty_termios_copy_hw(&tty->termios, old_termios); } - } else { - spin_lock_irqsave(&priv->lock, flags); - priv->line_control &= ~(CH341_BIT_DTR | CH341_BIT_RTS); - spin_unlock_irqrestore(&priv->lock, flags); } - ch341_set_handshake(port->serial->dev, priv->line_control); + spin_lock_irqsave(&priv->lock, flags); + if (C_BAUD(tty) == B0) + priv->line_control &= ~(CH341_BIT_DTR | CH341_BIT_RTS); + else if (old_termios && (old_termios->c_cflag & CBAUD) == B0) + priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); + spin_unlock_irqrestore(&priv->lock, flags); + ch341_set_handshake(port->serial->dev, priv->line_control); } static void ch341_break_ctl(struct tty_struct *tty, int break_state) From f2950b78547ffb8475297ada6b92bc2d774d5461 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Jan 2017 19:15:13 +0100 Subject: [PATCH 04/12] USB: serial: ch341: fix open error handling Make sure to stop the interrupt URB before returning on errors during open. Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 0cc5056b304d..8f41d4385f1c 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -319,7 +319,7 @@ static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port) r = ch341_configure(serial->dev, priv); if (r) - goto out; + return r; if (tty) ch341_set_termios(tty, port, NULL); @@ -329,12 +329,19 @@ static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port) if (r) { dev_err(&port->dev, "%s - failed to submit interrupt urb: %d\n", __func__, r); - goto out; + return r; } r = usb_serial_generic_open(tty, port); + if (r) + goto err_kill_interrupt_urb; -out: return r; + return 0; + +err_kill_interrupt_urb: + usb_kill_urb(port->interrupt_in_urb); + + return r; } /* Old_termios contains the original termios settings and From ce5e292828117d1b71cbd3edf9e9137cf31acd30 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Jan 2017 19:15:14 +0100 Subject: [PATCH 05/12] USB: serial: ch341: fix resume after reset Fix reset-resume handling which failed to resubmit the read and interrupt URBs, thereby leaving a port that was open before suspend in a broken state until closed and reopened. Fixes: 1ded7ea47b88 ("USB: ch341 serial: fix port number changed after resume") Fixes: 2bfd1c96a9fb ("USB: serial: ch341: remove reset_resume callback") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 8f41d4385f1c..5343d65f3b52 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -582,14 +582,23 @@ static int ch341_tiocmget(struct tty_struct *tty) static int ch341_reset_resume(struct usb_serial *serial) { - struct ch341_private *priv; - - priv = usb_get_serial_port_data(serial->port[0]); + struct usb_serial_port *port = serial->port[0]; + struct ch341_private *priv = usb_get_serial_port_data(port); + int ret; /* reconfigure ch341 serial port after bus-reset */ ch341_configure(serial->dev, priv); - return 0; + if (tty_port_initialized(&port->port)) { + ret = usb_submit_urb(port->interrupt_in_urb, GFP_NOIO); + if (ret) { + dev_err(&port->dev, "failed to submit interrupt urb: %d\n", + ret); + return ret; + } + } + + return usb_serial_generic_resume(serial); } static struct usb_serial_driver ch341_device = { From 3cca8624b6624e7ffb87dcd8a0a05bef9b50e97b Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Jan 2017 19:15:15 +0100 Subject: [PATCH 06/12] USB: serial: ch341: fix line settings after reset-resume A recent change added support for modifying the default line-control settings, but did not make sure that the modified settings were used as part of reconfiguration after a device has been reset during resume. This caused a port that was open before suspend to be unusable until being closed and reopened. Fixes: ba781bdf8662 ("USB: serial: ch341: add support for parity, frame length, stop bits") Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 5343d65f3b52..eabdd05a2147 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -95,6 +95,7 @@ struct ch341_private { unsigned baud_rate; /* set baud rate */ u8 line_control; /* set line control value RTS/DTR */ u8 line_status; /* active status of modem control inputs */ + u8 lcr; }; static void ch341_set_termios(struct tty_struct *tty, @@ -232,7 +233,7 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - r = ch341_init_set_baudrate(dev, priv, 0); + r = ch341_init_set_baudrate(dev, priv, priv->lcr); if (r < 0) goto out; @@ -397,6 +398,8 @@ static void ch341_set_termios(struct tty_struct *tty, if (r < 0 && old_termios) { priv->baud_rate = tty_termios_baud_rate(old_termios); tty_termios_copy_hw(&tty->termios, old_termios); + } else if (r == 0) { + priv->lcr = ctrl; } } From 55fa15b5987db22b4f35d3f0798928c126be5f1c Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Jan 2017 19:15:16 +0100 Subject: [PATCH 07/12] USB: serial: ch341: fix baud rate and line-control handling Revert to using direct register writes to set the divisor and line-control registers. A recent change switched to using the init vendor command to update these registers, something which also enabled support for CH341A devices. It turns out that simply setting bit 7 in the divisor register is sufficient to support CH341A and specifically prevent data from being buffered until a full endpoint-size packet (32 bytes) has been received. Using the init command also had the side-effect of temporarily deasserting the DTR/RTS signals on every termios change (including initialisation on open) something which for example could cause problems in setups where DTR is used to trigger a reset. Fixes: 4e46c410e050 ("USB: serial: ch341: reinitialize chip on reconfiguration") Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index eabdd05a2147..8d7b0847109b 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -133,8 +133,8 @@ static int ch341_control_in(struct usb_device *dev, return r; } -static int ch341_init_set_baudrate(struct usb_device *dev, - struct ch341_private *priv, unsigned ctrl) +static int ch341_set_baudrate_lcr(struct usb_device *dev, + struct ch341_private *priv, u8 lcr) { short a; int r; @@ -157,9 +157,19 @@ static int ch341_init_set_baudrate(struct usb_device *dev, factor = 0x10000 - factor; a = (factor & 0xff00) | divisor; - /* 0x9c is "enable SFR_UART Control register and timer" */ - r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT, - 0x9c | (ctrl << 8), a | 0x80); + /* + * CH341A buffers data until a full endpoint-size packet (32 bytes) + * has been received unless bit 7 is set. + */ + a |= BIT(7); + + r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x1312, a); + if (r) + return r; + + r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, lcr); + if (r) + return r; return r; } @@ -233,7 +243,7 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - r = ch341_init_set_baudrate(dev, priv, priv->lcr); + r = ch341_set_baudrate_lcr(dev, priv, priv->lcr); if (r < 0) goto out; @@ -394,7 +404,7 @@ static void ch341_set_termios(struct tty_struct *tty, if (baud_rate) { priv->baud_rate = baud_rate; - r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl); + r = ch341_set_baudrate_lcr(port->serial->dev, priv, ctrl); if (r < 0 && old_termios) { priv->baud_rate = tty_termios_baud_rate(old_termios); tty_termios_copy_hw(&tty->termios, old_termios); From 146cc8a17a3b4996f6805ee5c080e7101277c410 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 10 Jan 2017 12:05:37 +0100 Subject: [PATCH 08/12] USB: serial: kl5kusb105: fix line-state error handling The current implementation failed to detect short transfers when attempting to read the line state, and also, to make things worse, logged the content of the uninitialised heap transfer buffer. Fixes: abf492e7b3ae ("USB: kl5kusb105: fix DMA buffers on stack") Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/kl5kusb105.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c index 0ee190fc1bf8..6cb45757818f 100644 --- a/drivers/usb/serial/kl5kusb105.c +++ b/drivers/usb/serial/kl5kusb105.c @@ -192,10 +192,11 @@ static int klsi_105_get_line_state(struct usb_serial_port *port, status_buf, KLSI_STATUSBUF_LEN, 10000 ); - if (rc < 0) - dev_err(&port->dev, "Reading line status failed (error = %d)\n", - rc); - else { + if (rc != KLSI_STATUSBUF_LEN) { + dev_err(&port->dev, "reading line status failed: %d\n", rc); + if (rc >= 0) + rc = -EIO; + } else { status = get_unaligned_le16(status_buf); dev_info(&port->serial->dev->dev, "read status %x %x\n", From 620f1a632ebcc9811c2f8009ba52297c7006f805 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Tue, 13 Dec 2016 18:50:13 -0800 Subject: [PATCH 09/12] wusbcore: Fix one more crypto-on-the-stack bug The driver put a constant buffer of all zeros on the stack and pointed a scatterlist entry at it. This doesn't work with virtual stacks. Use ZERO_PAGE instead. Cc: stable@vger.kernel.org # 4.9 only Reported-by: Eric Biggers Signed-off-by: Andy Lutomirski Signed-off-by: Greg Kroah-Hartman --- drivers/usb/wusbcore/crypto.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c index 79451f7ef1b7..062c205f0046 100644 --- a/drivers/usb/wusbcore/crypto.c +++ b/drivers/usb/wusbcore/crypto.c @@ -216,7 +216,6 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, struct scatterlist sg[4], sg_dst; void *dst_buf; size_t dst_size; - const u8 bzero[16] = { 0 }; u8 iv[crypto_skcipher_ivsize(tfm_cbc)]; size_t zero_padding; @@ -261,7 +260,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, sg_set_buf(&sg[1], &scratch->b1, sizeof(scratch->b1)); sg_set_buf(&sg[2], b, blen); /* 0 if well behaved :) */ - sg_set_buf(&sg[3], bzero, zero_padding); + sg_set_page(&sg[3], ZERO_PAGE(0), zero_padding, 0); sg_init_one(&sg_dst, dst_buf, dst_size); skcipher_request_set_tfm(req, tfm_cbc); From 7b6c1b4c0e1e44544aa18161dba6a741c080a7ef Mon Sep 17 00:00:00 2001 From: Bin Liu Date: Tue, 10 Jan 2017 10:46:00 -0600 Subject: [PATCH 10/12] usb: musb: fix runtime PM in debugfs MUSB driver now has runtime PM support, but the debugfs driver misses the PM _get/_put() calls, which could cause MUSB register access failure. Cc: stable@vger.kernel.org # 4.9+ Acked-by: Tony Lindgren Signed-off-by: Bin Liu Signed-off-by: Greg Kroah-Hartman --- drivers/usb/musb/musb_debugfs.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c index 4fef50e5c8c1..dd70c88419d2 100644 --- a/drivers/usb/musb/musb_debugfs.c +++ b/drivers/usb/musb/musb_debugfs.c @@ -114,6 +114,7 @@ static int musb_regdump_show(struct seq_file *s, void *unused) unsigned i; seq_printf(s, "MUSB (M)HDRC Register Dump\n"); + pm_runtime_get_sync(musb->controller); for (i = 0; i < ARRAY_SIZE(musb_regmap); i++) { switch (musb_regmap[i].size) { @@ -132,6 +133,8 @@ static int musb_regdump_show(struct seq_file *s, void *unused) } } + pm_runtime_mark_last_busy(musb->controller); + pm_runtime_put_autosuspend(musb->controller); return 0; } @@ -145,7 +148,10 @@ static int musb_test_mode_show(struct seq_file *s, void *unused) struct musb *musb = s->private; unsigned test; + pm_runtime_get_sync(musb->controller); test = musb_readb(musb->mregs, MUSB_TESTMODE); + pm_runtime_mark_last_busy(musb->controller); + pm_runtime_put_autosuspend(musb->controller); if (test & MUSB_TEST_FORCE_HOST) seq_printf(s, "force host\n"); @@ -194,11 +200,12 @@ static ssize_t musb_test_mode_write(struct file *file, u8 test; char buf[18]; + pm_runtime_get_sync(musb->controller); test = musb_readb(musb->mregs, MUSB_TESTMODE); if (test) { dev_err(musb->controller, "Error: test mode is already set. " "Please do USB Bus Reset to start a new test.\n"); - return count; + goto ret; } memset(buf, 0x00, sizeof(buf)); @@ -234,6 +241,9 @@ static ssize_t musb_test_mode_write(struct file *file, musb_writeb(musb->mregs, MUSB_TESTMODE, test); +ret: + pm_runtime_mark_last_busy(musb->controller); + pm_runtime_put_autosuspend(musb->controller); return count; } @@ -254,8 +264,13 @@ static int musb_softconnect_show(struct seq_file *s, void *unused) switch (musb->xceiv->otg->state) { case OTG_STATE_A_HOST: case OTG_STATE_A_WAIT_BCON: + pm_runtime_get_sync(musb->controller); + reg = musb_readb(musb->mregs, MUSB_DEVCTL); connect = reg & MUSB_DEVCTL_SESSION ? 1 : 0; + + pm_runtime_mark_last_busy(musb->controller); + pm_runtime_put_autosuspend(musb->controller); break; default: connect = -1; @@ -284,6 +299,7 @@ static ssize_t musb_softconnect_write(struct file *file, if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count))) return -EFAULT; + pm_runtime_get_sync(musb->controller); if (!strncmp(buf, "0", 1)) { switch (musb->xceiv->otg->state) { case OTG_STATE_A_HOST: @@ -314,6 +330,8 @@ static ssize_t musb_softconnect_write(struct file *file, } } + pm_runtime_mark_last_busy(musb->controller); + pm_runtime_put_autosuspend(musb->controller); return count; } From 2d5a9c72d0c4ac73cf97f4b7814ed6c44b1e49ae Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Jan 2017 19:15:18 +0100 Subject: [PATCH 11/12] USB: serial: ch341: fix control-message error handling A short control transfer would currently fail to be detected, something which could lead to stale buffer data being used as valid input. Check for short transfers, and make sure to log any transfer errors. Note that this also avoids leaking heap data to user space (TIOCMGET) and the remote device (break control). Fixes: 6ce76104781a ("USB: Driver for CH341 USB-serial adaptor") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 8d7b0847109b..95aa5233726c 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -113,6 +113,8 @@ static int ch341_control_out(struct usb_device *dev, u8 request, r = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, value, index, NULL, 0, DEFAULT_TIMEOUT); + if (r < 0) + dev_err(&dev->dev, "failed to send control message: %d\n", r); return r; } @@ -130,7 +132,20 @@ static int ch341_control_in(struct usb_device *dev, r = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, value, index, buf, bufsize, DEFAULT_TIMEOUT); - return r; + if (r < bufsize) { + if (r >= 0) { + dev_err(&dev->dev, + "short control message received (%d < %u)\n", + r, bufsize); + r = -EIO; + } + + dev_err(&dev->dev, "failed to receive control message: %d\n", + r); + return r; + } + + return 0; } static int ch341_set_baudrate_lcr(struct usb_device *dev, @@ -181,9 +196,9 @@ static int ch341_set_handshake(struct usb_device *dev, u8 control) static int ch341_get_status(struct usb_device *dev, struct ch341_private *priv) { + const unsigned int size = 2; char *buffer; int r; - const unsigned size = 8; unsigned long flags; buffer = kmalloc(size, GFP_KERNEL); @@ -194,14 +209,9 @@ static int ch341_get_status(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - /* setup the private status if available */ - if (r == 2) { - r = 0; - spin_lock_irqsave(&priv->lock, flags); - priv->line_status = (~(*buffer)) & CH341_BITS_MODEM_STAT; - spin_unlock_irqrestore(&priv->lock, flags); - } else - r = -EPROTO; + spin_lock_irqsave(&priv->lock, flags); + priv->line_status = (~(*buffer)) & CH341_BITS_MODEM_STAT; + spin_unlock_irqrestore(&priv->lock, flags); out: kfree(buffer); return r; @@ -211,9 +221,9 @@ out: kfree(buffer); static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) { + const unsigned int size = 2; char *buffer; int r; - const unsigned size = 8; buffer = kmalloc(size, GFP_KERNEL); if (!buffer) From d6169d04097fd9ddf811e63eae4e5cd71e6666e2 Mon Sep 17 00:00:00 2001 From: Mathias Nyman Date: Wed, 11 Jan 2017 17:10:34 +0200 Subject: [PATCH 12/12] xhci: fix deadlock at host remove by running watchdog correctly If a URB is killed while the host is removed we can end up in a situation where the hub thread takes the roothub device lock, and waits for the URB to be given back by xhci-hcd, blocking the host remove code. xhci-hcd tries to stop the endpoint and give back the urb, but can't as the host is removed from PCI bus at the same time, preventing the normal way of giving back urb. Instead we need to rely on the stop command timeout function to give back the urb. This xhci_stop_endpoint_command_watchdog() timeout function used a XHCI_STATE_DYING flag to indicate if the timeout function is already running, but later this flag has been taking into use in other places to mark that xhci is dying. Remove checks for XHCI_STATE_DYING in xhci_urb_dequeue. We are still checking that reading from pci state does not return 0xffffffff or that host is not halted before trying to stop the endpoint. This whole area of stopping endpoints, giving back URBs, and the wathdog timeout need rework, this fix focuses on solving a specific deadlock issue that we can then send to stable before any major rework. Cc: Signed-off-by: Mathias Nyman Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/xhci-ring.c | 11 ----------- drivers/usb/host/xhci.c | 13 ------------- 2 files changed, 24 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 25f522b09dd9..e32029a31ca4 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -913,17 +913,6 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) spin_lock_irqsave(&xhci->lock, flags); ep->stop_cmds_pending--; - if (xhci->xhc_state & XHCI_STATE_REMOVING) { - spin_unlock_irqrestore(&xhci->lock, flags); - return; - } - if (xhci->xhc_state & XHCI_STATE_DYING) { - xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, - "Stop EP timer ran, but another timer marked " - "xHCI as DYING, exiting."); - spin_unlock_irqrestore(&xhci->lock, flags); - return; - } if (!(ep->stop_cmds_pending == 0 && (ep->ep_state & EP_HALT_PENDING))) { xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Stop EP timer ran, but no command pending, " diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 0c8deb9ed42d..9a0ec116654a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1534,19 +1534,6 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) xhci_urb_free_priv(urb_priv); return ret; } - if ((xhci->xhc_state & XHCI_STATE_DYING) || - (xhci->xhc_state & XHCI_STATE_HALTED)) { - xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, - "Ep 0x%x: URB %p to be canceled on " - "non-responsive xHCI host.", - urb->ep->desc.bEndpointAddress, urb); - /* Let the stop endpoint command watchdog timer (which set this - * state) finish cleaning up the endpoint TD lists. We must - * have caught it in the middle of dropping a lock and giving - * back an URB. - */ - goto done; - } ep_index = xhci_get_endpoint_index(&urb->ep->desc); ep = &xhci->devs[urb->dev->slot_id]->eps[ep_index];