Skip to contentSkip to conversation

USB: serial: kl5kusb105: fix open error paths

PATCH In review Johan Hovold 1 participant 4 messages Nov 29, 2016 – Nov 30, 2016

Subsystems (3)
Stable branch USB USB serial
lore
Share & tools
lore

Johan Hovold

author maintainer
#0 lore raw

cover USB: serial: kl5kusb105: fix open error paths

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(-)
Signature · 1 line
2.7.3
author maintainer
#1 lore raw

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

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 <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/kl5kusb105.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)
drivers/usb/serial/kl5kusb105.c +24 -9
index fc5d3a791e08..6f29bfadbe33 100644
@@ -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;
 }
Signature · 1 line
2.7.3
author maintainer
#2 lore raw

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

In reply to Johan Hovold

From: Pan Bian <[email protected]>

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 <[email protected]>
[johan: rebase on prerequisite fix and amend commit message]
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/kl5kusb105.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/usb/serial/kl5kusb105.c +2 -0
index 6f29bfadbe33..0ee190fc1bf8 100644
@@ -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);
Signature · 1 line
2.7.3
author maintainer
#3 lore raw
In reply to Johan Hovold
Quoted 8 lines · 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.

Quoted 7 lines

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