[PATCH 0/2] USB: serial: kl5kusb105: fix open error paths

[PATCH 0/2] USB: serial: kl5kusb105: fix open error paths

[Johan Hovold]

Pan Bian found an issue with the kl5kusb105 open error handling, which
would not abort an open attempt when a vendor command to "enable read"
failed. 

Turns out there were more issues with this function, specifically any
urbs submitted would not be killed before returning on failures to
retrieve the line status.

I fixed up the latter and rebased the Pan Bian's patch on top.

Johan


Johan Hovold (1):
  USB: serial: kl5kusb105: fix open error path

Pan Bian (1):
  USB: serial: kl5kusb105: abort on open exception path

 drivers/usb/serial/kl5kusb105.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

-- 
2.7.3

[PATCH 1/2] USB: serial: kl5kusb105: fix open error path

[Johan Hovold]

Kill urbs and disable read before returning from open on failure to
retrieve the line state.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kl5kusb105.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c
index fc5d3a791e08..6f29bfadbe33 100644
--- a/drivers/usb/serial/kl5kusb105.c
+++ b/drivers/usb/serial/kl5kusb105.c
@@ -296,7 +296,7 @@ static int  klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
 	rc = usb_serial_generic_open(tty, port);
 	if (rc) {
 		retval = rc;
-		goto exit;
+		goto err_free_cfg;
 	}
 
 	rc = usb_control_msg(port->serial->dev,
@@ -315,17 +315,32 @@ static int  klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
 		dev_dbg(&port->dev, "%s - enabled reading\n", __func__);
 
 	rc = klsi_105_get_line_state(port, &line_state);
-	if (rc >= 0) {
-		spin_lock_irqsave(&priv->lock, flags);
-		priv->line_state = line_state;
-		spin_unlock_irqrestore(&priv->lock, flags);
-		dev_dbg(&port->dev, "%s - read line state 0x%lx\n", __func__, line_state);
-		retval = 0;
-	} else
+	if (rc < 0) {
 		retval = rc;
+		goto err_disable_read;
+	}
+
+	spin_lock_irqsave(&priv->lock, flags);
+	priv->line_state = line_state;
+	spin_unlock_irqrestore(&priv->lock, flags);
+	dev_dbg(&port->dev, "%s - read line state 0x%lx\n", __func__,
+			line_state);
+
+	return 0;
 
-exit:
+err_disable_read:
+	usb_control_msg(port->serial->dev,
+			     usb_sndctrlpipe(port->serial->dev, 0),
+			     KL5KUSB105A_SIO_CONFIGURE,
+			     USB_TYPE_VENDOR | USB_DIR_OUT,
+			     KL5KUSB105A_SIO_CONFIGURE_READ_OFF,
+			     0, /* index */
+			     NULL, 0,
+			     KLSI_TIMEOUT);
+	usb_serial_generic_close(port);
+err_free_cfg:
 	kfree(cfg);
+
 	return retval;
 }
 
-- 
2.7.3

[PATCH 2/2] USB: serial: kl5kusb105: abort on open exception path

[Johan Hovold]

From: Pan Bian <bianpan2016@163.com>

Function klsi_105_open() calls usb_control_msg() (to "enable read") and
checks its return value. When the return value is unexpected, it only
assigns the error code to the return variable retval, but does not
terminate the exception path. This patch fixes the bug by inserting
"goto err_generic_close;" when the call to usb_control_msg() fails.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Pan Bian <bianpan2016@163.com>
[johan: rebase on prerequisite fix and amend commit message]
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kl5kusb105.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c
index 6f29bfadbe33..0ee190fc1bf8 100644
--- a/drivers/usb/serial/kl5kusb105.c
+++ b/drivers/usb/serial/kl5kusb105.c
@@ -311,6 +311,7 @@ static int  klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
 	if (rc < 0) {
 		dev_err(&port->dev, "Enabling read failed (error = %d)\n", rc);
 		retval = rc;
+		goto err_generic_close;
 	} else
 		dev_dbg(&port->dev, "%s - enabled reading\n", __func__);
 
@@ -337,6 +338,7 @@ static int  klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
 			     0, /* index */
 			     NULL, 0,
 			     KLSI_TIMEOUT);
+err_generic_close:
 	usb_serial_generic_close(port);
 err_free_cfg:
 	kfree(cfg);
-- 
2.7.3

Re: [PATCH 0/2] USB: serial: kl5kusb105: fix open error paths

[Johan Hovold]

On Tue, Nov 29, 2016 at 04:55:00PM +0100, Johan Hovold wrote:
> Pan Bian found an issue with the kl5kusb105 open error handling, which
> would not abort an open attempt when a vendor command to "enable read"
> failed. 
> 
> Turns out there were more issues with this function, specifically any
> urbs submitted would not be killed before returning on failures to
> retrieve the line status.
> 
> I fixed up the latter and rebased the Pan Bian's patch on top.

> Johan Hovold (1):
>   USB: serial: kl5kusb105: fix open error path
> 
> Pan Bian (1):
>   USB: serial: kl5kusb105: abort on open exception path

Now applied for -next.

Johan