From 3c77f7c9e96bc40ac6985dd595cdd551afd34f2e Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Sat, 9 Apr 2016 13:02:28 +0200 Subject: [PATCH 01/14] USB: serial: ftdi_sio: constify ftdi_sio_quirk structures The ftdi_sio_quirk structures are never modified, so declare them as const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall Signed-off-by: Johan Hovold --- drivers/usb/serial/ftdi_sio.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index 3a814e802dee..00820809139a 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -93,27 +93,27 @@ static int ftdi_8u2232c_probe(struct usb_serial *serial); static void ftdi_USB_UIRT_setup(struct ftdi_private *priv); static void ftdi_HE_TIRA1_setup(struct ftdi_private *priv); -static struct ftdi_sio_quirk ftdi_jtag_quirk = { +static const struct ftdi_sio_quirk ftdi_jtag_quirk = { .probe = ftdi_jtag_probe, }; -static struct ftdi_sio_quirk ftdi_NDI_device_quirk = { +static const struct ftdi_sio_quirk ftdi_NDI_device_quirk = { .probe = ftdi_NDI_device_setup, }; -static struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = { +static const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = { .port_probe = ftdi_USB_UIRT_setup, }; -static struct ftdi_sio_quirk ftdi_HE_TIRA1_quirk = { +static const struct ftdi_sio_quirk ftdi_HE_TIRA1_quirk = { .port_probe = ftdi_HE_TIRA1_setup, }; -static struct ftdi_sio_quirk ftdi_stmclite_quirk = { +static const struct ftdi_sio_quirk ftdi_stmclite_quirk = { .probe = ftdi_stmclite_probe, }; -static struct ftdi_sio_quirk ftdi_8u2232c_quirk = { +static const struct ftdi_sio_quirk ftdi_8u2232c_quirk = { .probe = ftdi_8u2232c_probe, }; @@ -1775,7 +1775,7 @@ static void remove_sysfs_attrs(struct usb_serial_port *port) static int ftdi_sio_probe(struct usb_serial *serial, const struct usb_device_id *id) { - struct ftdi_sio_quirk *quirk = + const struct ftdi_sio_quirk *quirk = (struct ftdi_sio_quirk *)id->driver_info; if (quirk && quirk->probe) { @@ -1792,7 +1792,7 @@ static int ftdi_sio_probe(struct usb_serial *serial, static int ftdi_sio_port_probe(struct usb_serial_port *port) { struct ftdi_private *priv; - struct ftdi_sio_quirk *quirk = usb_get_serial_data(port->serial); + const struct ftdi_sio_quirk *quirk = usb_get_serial_data(port->serial); priv = kzalloc(sizeof(struct ftdi_private), GFP_KERNEL); From 8c34d82e9dc67bb06e20e015ec677f82b72a26b3 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Wed, 20 Apr 2016 14:26:58 -0400 Subject: [PATCH 02/14] USB: serial: use IS_ENABLED() instead of checking for FOO || FOO_MODULE The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either built-in or as a module, use that macro instead of open coding the same. Signed-off-by: Javier Martinez Canillas Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 46f1f13b41f1..7ecf4ff86b9a 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -815,7 +815,7 @@ static int usb_serial_probe(struct usb_interface *interface, } } -#if defined(CONFIG_USB_SERIAL_PL2303) || defined(CONFIG_USB_SERIAL_PL2303_MODULE) +#if IS_ENABLED(CONFIG_USB_SERIAL_PL2303) /* BEGIN HORRIBLE HACK FOR PL2303 */ /* this is needed due to the looney way its endpoints are set up */ if (((le16_to_cpu(dev->descriptor.idVendor) == PL2303_VENDOR_ID) && From a377f9e906af4df9071ba8ddba60188cb4013d93 Mon Sep 17 00:00:00 2001 From: Konstantin Shkolnyy Date: Wed, 4 May 2016 16:56:52 -0500 Subject: [PATCH 03/14] USB: serial: cp210x: fix hardware flow-control disable A bug in the CRTSCTS handling caused RTS to alternate between CRTSCTS=0 => "RTS is transmit active signal" and CRTSCTS=1 => "RTS is used for receive flow control" instead of CRTSCTS=0 => "RTS is statically active" and CRTSCTS=1 => "RTS is used for receive flow control" This only happened after first having enabled CRTSCTS. Signed-off-by: Konstantin Shkolnyy Fixes: 39a66b8d22a3 ("[PATCH] USB: CP2101 Add support for flow control") Cc: stable [johan: reword commit message ] Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index dd47823bb014..fef7a512bff4 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -967,8 +967,7 @@ static void cp210x_set_termios(struct tty_struct *tty, } else { modem_ctl[0] &= ~0x7B; modem_ctl[0] |= 0x01; - /* FIXME - OR here instead of assignment looks wrong */ - modem_ctl[4] |= 0x40; + modem_ctl[4] = 0x40; dev_dbg(dev, "%s - flow control = NONE\n", __func__); } From 9034389cd81681b4f0123173eb836624199209c7 Mon Sep 17 00:00:00 2001 From: Konstantin Shkolnyy Date: Wed, 4 May 2016 16:57:02 -0500 Subject: [PATCH 04/14] USB: serial: cp210x: get rid of magic numbers in CRTSCTS flag code Replaced magic numbers used in the CRTSCTS flag code with symbolic names from the chip specification. Signed-off-by: Konstantin Shkolnyy Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 109 +++++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 25 deletions(-) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index fef7a512bff4..a33a4265125d 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -327,6 +327,42 @@ struct cp210x_comm_status { */ #define PURGE_ALL 0x000f +/* CP210X_GET_FLOW/CP210X_SET_FLOW read/write these 0x10 bytes */ +struct cp210x_flow_ctl { + __le32 ulControlHandshake; + __le32 ulFlowReplace; + __le32 ulXonLimit; + __le32 ulXoffLimit; +} __packed; + +/* cp210x_flow_ctl::ulControlHandshake */ +#define CP210X_SERIAL_DTR_MASK GENMASK(1, 0) +#define CP210X_SERIAL_DTR_SHIFT(_mode) (_mode) +#define CP210X_SERIAL_CTS_HANDSHAKE BIT(3) +#define CP210X_SERIAL_DSR_HANDSHAKE BIT(4) +#define CP210X_SERIAL_DCD_HANDSHAKE BIT(5) +#define CP210X_SERIAL_DSR_SENSITIVITY BIT(6) + +/* values for cp210x_flow_ctl::ulControlHandshake::CP210X_SERIAL_DTR_MASK */ +#define CP210X_SERIAL_DTR_INACTIVE 0 +#define CP210X_SERIAL_DTR_ACTIVE 1 +#define CP210X_SERIAL_DTR_FLOW_CTL 2 + +/* cp210x_flow_ctl::ulFlowReplace */ +#define CP210X_SERIAL_AUTO_TRANSMIT BIT(0) +#define CP210X_SERIAL_AUTO_RECEIVE BIT(1) +#define CP210X_SERIAL_ERROR_CHAR BIT(2) +#define CP210X_SERIAL_NULL_STRIPPING BIT(3) +#define CP210X_SERIAL_BREAK_CHAR BIT(4) +#define CP210X_SERIAL_RTS_MASK GENMASK(7, 6) +#define CP210X_SERIAL_RTS_SHIFT(_mode) (_mode << 6) +#define CP210X_SERIAL_XOFF_CONTINUE BIT(31) + +/* values for cp210x_flow_ctl::ulFlowReplace::CP210X_SERIAL_RTS_MASK */ +#define CP210X_SERIAL_RTS_INACTIVE 0 +#define CP210X_SERIAL_RTS_ACTIVE 1 +#define CP210X_SERIAL_RTS_FLOW_CTL 2 + /* * Reads a variable-sized block of CP210X_ registers, identified by req. * Returns data into buf in native USB byte order. @@ -694,9 +730,10 @@ static void cp210x_get_termios_port(struct usb_serial_port *port, { struct device *dev = &port->dev; unsigned int cflag; - u8 modem_ctl[16]; + struct cp210x_flow_ctl flow_ctl; u32 baud; u16 bits; + u32 ctl_hs; cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, &baud); @@ -792,9 +829,10 @@ static void cp210x_get_termios_port(struct usb_serial_port *port, break; } - cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl, - sizeof(modem_ctl)); - if (modem_ctl[0] & 0x08) { + cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl, + sizeof(flow_ctl)); + ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); + if (ctl_hs & CP210X_SERIAL_CTS_HANDSHAKE) { dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__); cflag |= CRTSCTS; } else { @@ -863,7 +901,6 @@ static void cp210x_set_termios(struct tty_struct *tty, struct device *dev = &port->dev; unsigned int cflag, old_cflag; u16 bits; - u8 modem_ctl[16]; cflag = tty->termios.c_cflag; old_cflag = old_termios->c_cflag; @@ -947,34 +984,56 @@ static void cp210x_set_termios(struct tty_struct *tty, } if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) { + struct cp210x_flow_ctl flow_ctl; + u32 ctl_hs; + u32 flow_repl; - /* Only bytes 0, 4 and 7 out of first 8 have functional bits */ - - cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl, - sizeof(modem_ctl)); - dev_dbg(dev, "%s - read modem controls = %02x .. .. .. %02x .. .. %02x\n", - __func__, modem_ctl[0], modem_ctl[4], modem_ctl[7]); + cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl, + sizeof(flow_ctl)); + ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); + flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace); + dev_dbg(dev, "%s - read ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n", + __func__, ctl_hs, flow_repl); if (cflag & CRTSCTS) { - modem_ctl[0] &= ~0x7B; - modem_ctl[0] |= 0x09; - modem_ctl[4] = 0x80; - /* FIXME - why clear reserved bits just read? */ - modem_ctl[5] = 0; - modem_ctl[6] = 0; - modem_ctl[7] = 0; + ctl_hs &= ~(CP210X_SERIAL_DTR_MASK | + CP210X_SERIAL_CTS_HANDSHAKE | + CP210X_SERIAL_DSR_HANDSHAKE | + CP210X_SERIAL_DCD_HANDSHAKE | + CP210X_SERIAL_DSR_SENSITIVITY); + ctl_hs |= CP210X_SERIAL_DTR_SHIFT( + CP210X_SERIAL_DTR_ACTIVE); + ctl_hs |= CP210X_SERIAL_CTS_HANDSHAKE; + /* + * FIXME: Why clear bits unrelated to flow control. + * Why clear CP210X_SERIAL_XOFF_CONTINUE which is + * never set + */ + flow_repl = 0; + flow_repl |= CP210X_SERIAL_RTS_SHIFT( + CP210X_SERIAL_RTS_FLOW_CTL); dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__); } else { - modem_ctl[0] &= ~0x7B; - modem_ctl[0] |= 0x01; - modem_ctl[4] = 0x40; + ctl_hs &= ~(CP210X_SERIAL_DTR_MASK | + CP210X_SERIAL_CTS_HANDSHAKE | + CP210X_SERIAL_DSR_HANDSHAKE | + CP210X_SERIAL_DCD_HANDSHAKE | + CP210X_SERIAL_DSR_SENSITIVITY); + ctl_hs |= CP210X_SERIAL_DTR_SHIFT( + CP210X_SERIAL_DTR_ACTIVE); + /* FIXME: Why clear bits unrelated to flow control */ + flow_repl &= 0xffffff00; + flow_repl |= CP210X_SERIAL_RTS_SHIFT( + CP210X_SERIAL_RTS_ACTIVE); dev_dbg(dev, "%s - flow control = NONE\n", __func__); } - dev_dbg(dev, "%s - write modem controls = %02x .. .. .. %02x .. .. %02x\n", - __func__, modem_ctl[0], modem_ctl[4], modem_ctl[7]); - cp210x_write_reg_block(port, CP210X_SET_FLOW, modem_ctl, - sizeof(modem_ctl)); + dev_dbg(dev, "%s - write ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n", + __func__, ctl_hs, flow_repl); + flow_ctl.ulControlHandshake = cpu_to_le32(ctl_hs); + flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl); + cp210x_write_reg_block(port, CP210X_SET_FLOW, &flow_ctl, + sizeof(flow_ctl)); } } From ab5701ada2473b111c24ca7f4360b0cdb5badb60 Mon Sep 17 00:00:00 2001 From: Konstantin Shkolnyy Date: Wed, 4 May 2016 16:57:11 -0500 Subject: [PATCH 05/14] USB: serial: cp210x: clean up CRTSCTS flag code The CRTSCTS flag code cleared (and inconsistently) bits unrelated to CRTSCTS functionality. It was also harder than necessary to read. Signed-off-by: Konstantin Shkolnyy Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index a33a4265125d..df1808201207 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -995,34 +995,22 @@ static void cp210x_set_termios(struct tty_struct *tty, dev_dbg(dev, "%s - read ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n", __func__, ctl_hs, flow_repl); + ctl_hs &= ~CP210X_SERIAL_DSR_HANDSHAKE; + ctl_hs &= ~CP210X_SERIAL_DCD_HANDSHAKE; + ctl_hs &= ~CP210X_SERIAL_DSR_SENSITIVITY; + ctl_hs &= ~CP210X_SERIAL_DTR_MASK; + ctl_hs |= CP210X_SERIAL_DTR_SHIFT(CP210X_SERIAL_DTR_ACTIVE); if (cflag & CRTSCTS) { - ctl_hs &= ~(CP210X_SERIAL_DTR_MASK | - CP210X_SERIAL_CTS_HANDSHAKE | - CP210X_SERIAL_DSR_HANDSHAKE | - CP210X_SERIAL_DCD_HANDSHAKE | - CP210X_SERIAL_DSR_SENSITIVITY); - ctl_hs |= CP210X_SERIAL_DTR_SHIFT( - CP210X_SERIAL_DTR_ACTIVE); ctl_hs |= CP210X_SERIAL_CTS_HANDSHAKE; - /* - * FIXME: Why clear bits unrelated to flow control. - * Why clear CP210X_SERIAL_XOFF_CONTINUE which is - * never set - */ - flow_repl = 0; + + flow_repl &= ~CP210X_SERIAL_RTS_MASK; flow_repl |= CP210X_SERIAL_RTS_SHIFT( CP210X_SERIAL_RTS_FLOW_CTL); dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__); } else { - ctl_hs &= ~(CP210X_SERIAL_DTR_MASK | - CP210X_SERIAL_CTS_HANDSHAKE | - CP210X_SERIAL_DSR_HANDSHAKE | - CP210X_SERIAL_DCD_HANDSHAKE | - CP210X_SERIAL_DSR_SENSITIVITY); - ctl_hs |= CP210X_SERIAL_DTR_SHIFT( - CP210X_SERIAL_DTR_ACTIVE); - /* FIXME: Why clear bits unrelated to flow control */ - flow_repl &= 0xffffff00; + ctl_hs &= ~CP210X_SERIAL_CTS_HANDSHAKE; + + flow_repl &= ~CP210X_SERIAL_RTS_MASK; flow_repl |= CP210X_SERIAL_RTS_SHIFT( CP210X_SERIAL_RTS_ACTIVE); dev_dbg(dev, "%s - flow control = NONE\n", __func__); From c5c0c55598cefc826d6cfb0a417eeaee3631715c Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 8 May 2016 20:07:56 +0200 Subject: [PATCH 06/14] USB: serial: io_edgeport: fix memory leaks in attach error path Private data, URBs and buffers allocated for Epic devices during attach were never released on errors (e.g. missing endpoints). Fixes: 6e8cf7751f9f ("USB: add EPIC support to the io_edgeport driver") Cc: stable # v2.6.21 Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/io_edgeport.c | 39 +++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c index f3007ecdd1b4..edd568bc0de5 100644 --- a/drivers/usb/serial/io_edgeport.c +++ b/drivers/usb/serial/io_edgeport.c @@ -2849,14 +2849,16 @@ static int edge_startup(struct usb_serial *serial) /* not set up yet, so do it now */ edge_serial->interrupt_read_urb = usb_alloc_urb(0, GFP_KERNEL); - if (!edge_serial->interrupt_read_urb) - return -ENOMEM; + if (!edge_serial->interrupt_read_urb) { + response = -ENOMEM; + break; + } edge_serial->interrupt_in_buffer = kmalloc(buffer_size, GFP_KERNEL); if (!edge_serial->interrupt_in_buffer) { - usb_free_urb(edge_serial->interrupt_read_urb); - return -ENOMEM; + response = -ENOMEM; + break; } edge_serial->interrupt_in_endpoint = endpoint->bEndpointAddress; @@ -2884,14 +2886,16 @@ static int edge_startup(struct usb_serial *serial) /* not set up yet, so do it now */ edge_serial->read_urb = usb_alloc_urb(0, GFP_KERNEL); - if (!edge_serial->read_urb) - return -ENOMEM; + if (!edge_serial->read_urb) { + response = -ENOMEM; + break; + } edge_serial->bulk_in_buffer = kmalloc(buffer_size, GFP_KERNEL); if (!edge_serial->bulk_in_buffer) { - usb_free_urb(edge_serial->read_urb); - return -ENOMEM; + response = -ENOMEM; + break; } edge_serial->bulk_in_endpoint = endpoint->bEndpointAddress; @@ -2917,9 +2921,22 @@ static int edge_startup(struct usb_serial *serial) } } - if (!interrupt_in_found || !bulk_in_found || !bulk_out_found) { - dev_err(ddev, "Error - the proper endpoints were not found!\n"); - return -ENODEV; + if (response || !interrupt_in_found || !bulk_in_found || + !bulk_out_found) { + if (!response) { + dev_err(ddev, "expected endpoints not found\n"); + response = -ENODEV; + } + + usb_free_urb(edge_serial->interrupt_read_urb); + kfree(edge_serial->interrupt_in_buffer); + + usb_free_urb(edge_serial->read_urb); + kfree(edge_serial->bulk_in_buffer); + + kfree(edge_serial); + + return response; } /* start interrupt read for this edgeport this interrupt will From c8d62957d450cc1a22ce3242908709fe367ddc8e Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 8 May 2016 20:07:57 +0200 Subject: [PATCH 07/14] USB: serial: io_edgeport: fix memory leaks in probe error path URBs and buffers allocated in attach for Epic devices would never be deallocated in case of a later probe error (e.g. failure to allocate minor numbers) as disconnect is then never called. Fix by moving deallocation to release and making sure that the URBs are first unlinked. Fixes: f9c99bb8b3a1 ("USB: usb-serial: replace shutdown with disconnect, release") Cc: stable # v2.6.31 Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/io_edgeport.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c index edd568bc0de5..11c05ce2f35f 100644 --- a/drivers/usb/serial/io_edgeport.c +++ b/drivers/usb/serial/io_edgeport.c @@ -2959,16 +2959,9 @@ static void edge_disconnect(struct usb_serial *serial) { struct edgeport_serial *edge_serial = usb_get_serial_data(serial); - /* stop reads and writes on all ports */ - /* free up our endpoint stuff */ if (edge_serial->is_epic) { usb_kill_urb(edge_serial->interrupt_read_urb); - usb_free_urb(edge_serial->interrupt_read_urb); - kfree(edge_serial->interrupt_in_buffer); - usb_kill_urb(edge_serial->read_urb); - usb_free_urb(edge_serial->read_urb); - kfree(edge_serial->bulk_in_buffer); } } @@ -2981,6 +2974,16 @@ static void edge_release(struct usb_serial *serial) { struct edgeport_serial *edge_serial = usb_get_serial_data(serial); + if (edge_serial->is_epic) { + usb_kill_urb(edge_serial->interrupt_read_urb); + usb_free_urb(edge_serial->interrupt_read_urb); + kfree(edge_serial->interrupt_in_buffer); + + usb_kill_urb(edge_serial->read_urb); + usb_free_urb(edge_serial->read_urb); + kfree(edge_serial->bulk_in_buffer); + } + kfree(edge_serial); } From 35be1a71d70775e7bd7e45fa6d2897342ff4c9d2 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 8 May 2016 20:07:58 +0200 Subject: [PATCH 08/14] USB: serial: keyspan: fix use-after-free in probe error path The interface instat and indat URBs were submitted in attach, but never unlinked in release before deallocating the corresponding transfer buffers. In the case of a late probe error (e.g. due to failed minor allocation), disconnect would not have been called before release, causing the buffers to be freed while the URBs are still in use. We'd also end up with active URBs for an unbound interface. Fixes: f9c99bb8b3a1 ("USB: usb-serial: replace shutdown with disconnect, release") Cc: stable # v2.6.31 Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/keyspan.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c index b6bd8e4a6486..1cf05883f48c 100644 --- a/drivers/usb/serial/keyspan.c +++ b/drivers/usb/serial/keyspan.c @@ -2376,6 +2376,10 @@ static void keyspan_release(struct usb_serial *serial) s_priv = usb_get_serial_data(serial); + /* Make sure to unlink the URBs submitted in attach. */ + usb_kill_urb(s_priv->instat_urb); + usb_kill_urb(s_priv->indat_urb); + usb_free_urb(s_priv->instat_urb); usb_free_urb(s_priv->indat_urb); usb_free_urb(s_priv->glocont_urb); From 61924505ae0037527879446b36ac27c60210bc77 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 8 May 2016 20:07:59 +0200 Subject: [PATCH 09/14] USB: serial: keyspan: fix URB unlink A driver must not rely on the URB status field to try to determine if an URB is active. Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/keyspan.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c index 1cf05883f48c..86d54932843d 100644 --- a/drivers/usb/serial/keyspan.c +++ b/drivers/usb/serial/keyspan.c @@ -1082,12 +1082,6 @@ static int keyspan_open(struct tty_struct *tty, struct usb_serial_port *port) return 0; } -static inline void stop_urb(struct urb *urb) -{ - if (urb && urb->status == -EINPROGRESS) - usb_kill_urb(urb); -} - static void keyspan_dtr_rts(struct usb_serial_port *port, int on) { struct keyspan_port_private *p_priv = usb_get_serial_port_data(port); @@ -1114,10 +1108,10 @@ static void keyspan_close(struct usb_serial_port *port) p_priv->out_flip = 0; p_priv->in_flip = 0; - stop_urb(p_priv->inack_urb); + usb_kill_urb(p_priv->inack_urb); for (i = 0; i < 2; i++) { - stop_urb(p_priv->in_urbs[i]); - stop_urb(p_priv->out_urbs[i]); + usb_kill_urb(p_priv->in_urbs[i]); + usb_kill_urb(p_priv->out_urbs[i]); } } @@ -2365,9 +2359,9 @@ static void keyspan_disconnect(struct usb_serial *serial) s_priv = usb_get_serial_data(serial); - stop_urb(s_priv->instat_urb); - stop_urb(s_priv->glocont_urb); - stop_urb(s_priv->indat_urb); + usb_kill_urb(s_priv->instat_urb); + usb_kill_urb(s_priv->glocont_urb); + usb_kill_urb(s_priv->indat_urb); } static void keyspan_release(struct usb_serial *serial) @@ -2495,11 +2489,11 @@ static int keyspan_port_remove(struct usb_serial_port *port) p_priv = usb_get_serial_port_data(port); - stop_urb(p_priv->inack_urb); - stop_urb(p_priv->outcont_urb); + usb_kill_urb(p_priv->inack_urb); + usb_kill_urb(p_priv->outcont_urb); for (i = 0; i < 2; i++) { - stop_urb(p_priv->in_urbs[i]); - stop_urb(p_priv->out_urbs[i]); + usb_kill_urb(p_priv->in_urbs[i]); + usb_kill_urb(p_priv->out_urbs[i]); } usb_free_urb(p_priv->inack_urb); From 0cd782b0bea1917a522efaedf79429fd7f6621b9 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 8 May 2016 20:08:00 +0200 Subject: [PATCH 10/14] USB: serial: keyspan: fix debug and error messages The URB status is signed and should be printed using %d rather than %x. Also print endpoint addresses consistently using %x rather than %d, and merge a broken-up error message string. Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/keyspan.c | 42 +++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c index 86d54932843d..1f9414bdd649 100644 --- a/drivers/usb/serial/keyspan.c +++ b/drivers/usb/serial/keyspan.c @@ -255,7 +255,7 @@ static int keyspan_write(struct tty_struct *tty, return count; } - dev_dbg(&port->dev, "%s - endpoint %d flip %d\n", + dev_dbg(&port->dev, "%s - endpoint %x flip %d\n", __func__, usb_pipeendpoint(this_urb->pipe), flip); if (this_urb->status == -EINPROGRESS) { @@ -300,7 +300,7 @@ static void usa26_indat_callback(struct urb *urb) endpoint = usb_pipeendpoint(urb->pipe); if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x on endpoint %d.\n", + dev_dbg(&urb->dev->dev, "%s - nonzero status %d on endpoint %x\n", __func__, status, endpoint); return; } @@ -393,7 +393,8 @@ static void usa26_instat_callback(struct urb *urb) serial = urb->context; if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status); + dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n", + __func__, status); return; } if (urb->actual_length != 9) { @@ -452,7 +453,7 @@ static void usa28_indat_callback(struct urb *urb) do { if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x on endpoint %d.\n", + dev_dbg(&urb->dev->dev, "%s - nonzero status %d on endpoint %x\n", __func__, status, usb_pipeendpoint(urb->pipe)); return; } @@ -511,7 +512,8 @@ static void usa28_instat_callback(struct urb *urb) serial = urb->context; if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status); + dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n", + __func__, status); return; } @@ -591,7 +593,8 @@ static void usa49_instat_callback(struct urb *urb) serial = urb->context; if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status); + dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n", + __func__, status); return; } @@ -646,7 +649,7 @@ static void usa49_indat_callback(struct urb *urb) endpoint = usb_pipeendpoint(urb->pipe); if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x on endpoint %d.\n", + dev_dbg(&urb->dev->dev, "%s - nonzero status %d on endpoint %x\n", __func__, status, endpoint); return; } @@ -698,7 +701,8 @@ static void usa49wg_indat_callback(struct urb *urb) serial = urb->context; if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status); + dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n", + __func__, status); return; } @@ -774,8 +778,8 @@ static void usa90_indat_callback(struct urb *urb) endpoint = usb_pipeendpoint(urb->pipe); if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x on endpoint %d.\n", - __func__, status, endpoint); + dev_dbg(&urb->dev->dev, "%s - nonzero status %d on endpoint %x\n", + __func__, status, endpoint); return; } @@ -847,7 +851,8 @@ static void usa90_instat_callback(struct urb *urb) serial = urb->context; if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status); + dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n", + __func__, status); return; } if (urb->actual_length < 14) { @@ -912,7 +917,8 @@ static void usa67_instat_callback(struct urb *urb) serial = urb->context; if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status); + dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n", + __func__, status); return; } @@ -1215,8 +1221,8 @@ static struct usb_endpoint_descriptor const *find_ep(struct usb_serial const *se if (ep->bEndpointAddress == endpoint) return ep; } - dev_warn(&serial->interface->dev, "found no endpoint descriptor for " - "endpoint %x\n", endpoint); + dev_warn(&serial->interface->dev, "found no endpoint descriptor for endpoint %x\n", + endpoint); return NULL; } @@ -1231,7 +1237,8 @@ static struct urb *keyspan_setup_urb(struct usb_serial *serial, int endpoint, if (endpoint == -1) return NULL; /* endpoint not needed */ - dev_dbg(&serial->interface->dev, "%s - alloc for endpoint %d.\n", __func__, endpoint); + dev_dbg(&serial->interface->dev, "%s - alloc for endpoint %x\n", + __func__, endpoint); urb = usb_alloc_urb(0, GFP_KERNEL); /* No ISO */ if (!urb) return NULL; @@ -1566,7 +1573,8 @@ static int keyspan_usa26_send_setup(struct usb_serial *serial, return -1; } - dev_dbg(&port->dev, "%s - endpoint %d\n", __func__, usb_pipeendpoint(this_urb->pipe)); + dev_dbg(&port->dev, "%s - endpoint %x\n", + __func__, usb_pipeendpoint(this_urb->pipe)); /* Save reset port val for resend. Don't overwrite resend for open/close condition. */ @@ -1832,7 +1840,7 @@ static int keyspan_usa49_send_setup(struct usb_serial *serial, return -1; } - dev_dbg(&port->dev, "%s - endpoint %d (%d)\n", + dev_dbg(&port->dev, "%s - endpoint %x (%d)\n", __func__, usb_pipeendpoint(this_urb->pipe), device_port); /* Save reset port val for resend. From 9e45284984096314994777f27e1446dfbfd2f0d7 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 8 May 2016 20:08:01 +0200 Subject: [PATCH 11/14] USB: serial: mxuport: fix use-after-free in probe error path The interface read and event URBs are submitted in attach, but were never explicitly unlinked by the driver. Instead the URBs would have been killed by usb-serial core on disconnect. In case of a late probe error (e.g. due to failed minor allocation), disconnect is never called and we could end up with active URBs for an unbound interface. This in turn could lead to deallocated memory being dereferenced in the completion callbacks. Fixes: ee467a1f2066 ("USB: serial: add Moxa UPORT 12XX/14XX/16XX driver") Cc: stable # v3.14 Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/mxuport.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c index 31a8b47f1ac6..c6596cbcc4b6 100644 --- a/drivers/usb/serial/mxuport.c +++ b/drivers/usb/serial/mxuport.c @@ -1259,6 +1259,15 @@ static int mxuport_attach(struct usb_serial *serial) return 0; } +static void mxuport_release(struct usb_serial *serial) +{ + struct usb_serial_port *port0 = serial->port[0]; + struct usb_serial_port *port1 = serial->port[1]; + + usb_serial_generic_close(port1); + usb_serial_generic_close(port0); +} + static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port) { struct mxuport_port *mxport = usb_get_serial_port_data(port); @@ -1361,6 +1370,7 @@ static struct usb_serial_driver mxuport_device = { .probe = mxuport_probe, .port_probe = mxuport_port_probe, .attach = mxuport_attach, + .release = mxuport_release, .calc_num_ports = mxuport_calc_num_ports, .open = mxuport_open, .close = mxuport_close, From 028c49f5e02a257c94129cd815f7c8485f51d4ef Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 8 May 2016 20:08:02 +0200 Subject: [PATCH 12/14] USB: serial: quatech2: fix use-after-free in probe error path The interface read URB is submitted in attach, but was only unlinked by the driver at disconnect. In case of a late probe error (e.g. due to failed minor allocation), disconnect is never called and we would end up with active URBs for an unbound interface. This in turn could lead to deallocated memory being dereferenced in the completion callback. Fixes: f7a33e608d9a ("USB: serial: add quatech2 usb to serial driver") Cc: stable # v3.5: 40d04738491d Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/quatech2.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/serial/quatech2.c b/drivers/usb/serial/quatech2.c index 2df8ad5ede89..85acb50a7ee2 100644 --- a/drivers/usb/serial/quatech2.c +++ b/drivers/usb/serial/quatech2.c @@ -141,6 +141,7 @@ static void qt2_release(struct usb_serial *serial) serial_priv = usb_get_serial_data(serial); + usb_kill_urb(serial_priv->read_urb); usb_free_urb(serial_priv->read_urb); kfree(serial_priv->read_buffer); kfree(serial_priv); From 194e958c5c3bf4fa0805b0653fe2d0428d3791ff Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 8 May 2016 20:08:03 +0200 Subject: [PATCH 13/14] USB: serial: fix minor-number allocation Due to a missing upper bound, invalid minor numbers could be assigned to ports. Such devices would later fail to register, but let's catch this early as intended and avoid having devices with only a subset of their ports registered (potentially the empty set). Signed-off-by: Johan Hovold Acked-by: Greg Kroah-Hartman --- drivers/usb/serial/usb-serial.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 7ecf4ff86b9a..4d2b310de55d 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -96,7 +96,8 @@ static int allocate_minors(struct usb_serial *serial, int num_ports) mutex_lock(&table_lock); for (i = 0; i < num_ports; ++i) { port = serial->port[i]; - minor = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL); + minor = idr_alloc(&serial_minors, port, 0, + USB_SERIAL_TTY_MINORS, GFP_KERNEL); if (minor < 0) goto error; port->minor = minor; From b923c6c62981cec5e2d2187fd700c2fc4386fc45 Mon Sep 17 00:00:00 2001 From: Mathieu OTHACEHE Date: Tue, 10 May 2016 09:08:48 +0200 Subject: [PATCH 14/14] USB: serial: ti_usb_3410_5052: add MOXA UPORT 11x0 support Add support for : - UPort 1110 : 1 port RS-232 USB to Serial Hub. - UPort 1130 : 1 port RS-422/485 USB to Serial Hub. - UPort 1130I : 1 port RS-422/485 USB to Serial Hub with Isolation. - UPort 1150 : 1 port RS-232/422/485 USB to Serial Hub. - UPort 1150I : 1 port RS-232/422/485 USB to Serial Hub with Isolation. These devices are based on TI 3410 chip. Signed-off-by: Mathieu OTHACEHE [johan: fix rs485-only check ] Signed-off-by: Johan Hovold --- drivers/usb/serial/ti_usb_3410_5052.c | 55 +++++++++++++++++++++++++-- drivers/usb/serial/ti_usb_3410_5052.h | 8 ++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c index 2694df2f4559..e7dbbef2af2a 100644 --- a/drivers/usb/serial/ti_usb_3410_5052.c +++ b/drivers/usb/serial/ti_usb_3410_5052.c @@ -80,6 +80,7 @@ struct ti_device { int td_open_port_count; struct usb_serial *td_serial; int td_is_3410; + bool td_rs485_only; int td_urb_error; }; @@ -160,6 +161,11 @@ static const struct usb_device_id ti_id_table_3410[] = { { USB_DEVICE(ABBOTT_VENDOR_ID, ABBOTT_STRIP_PORT_ID) }, { USB_DEVICE(TI_VENDOR_ID, FRI2_PRODUCT_ID) }, { USB_DEVICE(HONEYWELL_VENDOR_ID, HONEYWELL_HGI80_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1110_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1130_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1131_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1150_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1151_PRODUCT_ID) }, { } /* terminator */ }; @@ -193,6 +199,11 @@ static const struct usb_device_id ti_id_table_combined[] = { { USB_DEVICE(ABBOTT_VENDOR_ID, ABBOTT_STRIP_PORT_ID) }, { USB_DEVICE(TI_VENDOR_ID, FRI2_PRODUCT_ID) }, { USB_DEVICE(HONEYWELL_VENDOR_ID, HONEYWELL_HGI80_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1110_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1130_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1131_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1150_PRODUCT_ID) }, + { USB_DEVICE(MXU1_VENDOR_ID, MXU1_1151_PRODUCT_ID) }, { } /* terminator */ }; @@ -277,6 +288,11 @@ MODULE_FIRMWARE("mts_gsm.fw"); MODULE_FIRMWARE("mts_edge.fw"); MODULE_FIRMWARE("mts_mt9234mu.fw"); MODULE_FIRMWARE("mts_mt9234zba.fw"); +MODULE_FIRMWARE("moxa/moxa-1110.fw"); +MODULE_FIRMWARE("moxa/moxa-1130.fw"); +MODULE_FIRMWARE("moxa/moxa-1131.fw"); +MODULE_FIRMWARE("moxa/moxa-1150.fw"); +MODULE_FIRMWARE("moxa/moxa-1151.fw"); module_param(closing_wait, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(closing_wait, @@ -292,6 +308,9 @@ static int ti_startup(struct usb_serial *serial) { struct ti_device *tdev; struct usb_device *dev = serial->dev; + struct usb_host_interface *cur_altsetting; + int num_endpoints; + u16 vid, pid; int status; dev_dbg(&dev->dev, @@ -315,8 +334,22 @@ static int ti_startup(struct usb_serial *serial) dev_dbg(&dev->dev, "%s - device type is %s\n", __func__, tdev->td_is_3410 ? "3410" : "5052"); - /* if we have only 1 configuration, download firmware */ - if (dev->descriptor.bNumConfigurations == 1) { + vid = le16_to_cpu(dev->descriptor.idVendor); + pid = le16_to_cpu(dev->descriptor.idProduct); + if (vid == MXU1_VENDOR_ID) { + switch (pid) { + case MXU1_1130_PRODUCT_ID: + case MXU1_1131_PRODUCT_ID: + tdev->td_rs485_only = true; + break; + } + } + + cur_altsetting = serial->interface->cur_altsetting; + num_endpoints = cur_altsetting->desc.bNumEndpoints; + + /* if we have only 1 configuration and 1 endpoint, download firmware */ + if (dev->descriptor.bNumConfigurations == 1 && num_endpoints == 1) { status = ti_download_firmware(tdev); if (status != 0) @@ -371,7 +404,11 @@ static int ti_port_probe(struct usb_serial_port *port) port->port.closing_wait = msecs_to_jiffies(10 * closing_wait); tport->tp_port = port; tport->tp_tdev = usb_get_serial_data(port->serial); - tport->tp_uart_mode = 0; /* default is RS232 */ + + if (tport->tp_tdev->td_rs485_only) + tport->tp_uart_mode = TI_UART_485_RECEIVER_DISABLED; + else + tport->tp_uart_mode = TI_UART_232; usb_set_serial_port_data(port, tport); @@ -1450,6 +1487,16 @@ static int ti_download_firmware(struct ti_device *tdev) const struct firmware *fw_p; char buf[32]; + if (le16_to_cpu(dev->descriptor.idVendor) == MXU1_VENDOR_ID) { + snprintf(buf, + sizeof(buf), + "moxa/moxa-%04x.fw", + le16_to_cpu(dev->descriptor.idProduct)); + + status = request_firmware(&fw_p, buf, &dev->dev); + goto check_firmware; + } + /* try ID specific firmware first, then try generic firmware */ sprintf(buf, "ti_usb-v%04x-p%04x.fw", le16_to_cpu(dev->descriptor.idVendor), @@ -1487,6 +1534,8 @@ static int ti_download_firmware(struct ti_device *tdev) } status = request_firmware(&fw_p, buf, &dev->dev); } + +check_firmware: if (status) { dev_err(&dev->dev, "%s - firmware not found\n", __func__); return -ENOENT; diff --git a/drivers/usb/serial/ti_usb_3410_5052.h b/drivers/usb/serial/ti_usb_3410_5052.h index 98f35c656c02..bbfd3a184600 100644 --- a/drivers/usb/serial/ti_usb_3410_5052.h +++ b/drivers/usb/serial/ti_usb_3410_5052.h @@ -60,6 +60,14 @@ #define HONEYWELL_VENDOR_ID 0x10ac #define HONEYWELL_HGI80_PRODUCT_ID 0x0102 /* Honeywell HGI80 */ +/* Moxa UPORT 11x0 vendor and product IDs */ +#define MXU1_VENDOR_ID 0x110a +#define MXU1_1110_PRODUCT_ID 0x1110 +#define MXU1_1130_PRODUCT_ID 0x1130 +#define MXU1_1131_PRODUCT_ID 0x1131 +#define MXU1_1150_PRODUCT_ID 0x1150 +#define MXU1_1151_PRODUCT_ID 0x1151 + /* Commands */ #define TI_GET_VERSION 0x01 #define TI_GET_PORT_STATUS 0x02