diff mbox series

[1/2] USB: serial: cp210x: fix dropped characters with CP2102

Message ID 20210922113100.20888-1-johan@kernel.org (mailing list archive)
State Superseded
Commit c32dfec6c1c36bbbcd5d33e949d99aeb215877ec
Headers show
Series [1/2] USB: serial: cp210x: fix dropped characters with CP2102 | expand

Commit Message

Johan Hovold Sept. 22, 2021, 11:30 a.m. UTC
Some CP2102 do not support event-insertion mode but return no error when
attempting to enable it.

This means that any event escape characters in the input stream will not
be escaped by the device and consequently regular data may be
interpreted as escape sequences and be removed from the stream by the
driver.

The reporter's device has batch number DCL00X etched into it and as
discovered by the SHA2017 Badge team, counterfeit devices with that
marking can be detected by sending malformed vendor requests. [1][2]

Tests confirm that the possibly counterfeit CP2102 returns a single byte
in response to a malformed two-byte part-number request, while an
original CP2102 returns two bytes. Assume that every CP2102 that behaves
this way also does not support event-insertion mode (e.g. cannot report
parity errors).

[1] https://mobile.twitter.com/sha2017badge/status/1167902087289532418
[2] https://hackaday.com/2017/08/14/hands-on-with-the-shacamp-2017-badge/#comment-3903376

Reported-by: Malte Di Donato <malte@neo-soft.org>
Fixes: a7207e9835a4 ("USB: serial: cp210x: add support for line-status events")
Cc: stable@vger.kernel.org	# 5.9
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

Malte Di Donato Sept. 22, 2021, 8:12 p.m. UTC | #1
Tested-by: Malte Di Donato <malte@neo-soft.org>

# dmesg (cp2102 connect)

[57018.586990] usb 3-1.2: new full-speed USB device number 17 using ehci-pci
[57018.727102] usb 3-1.2: New USB device found, idVendor=10c4, 
idProduct=ea60, bcdDevice= 1.00
[57018.727109] usb 3-1.2: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3
[57018.727112] usb 3-1.2: Product: CP2102 USB to UART Bridge Controller
[57018.727115] usb 3-1.2: Manufacturer: Silicon Labs
[57018.727117] usb 3-1.2: SerialNumber: 0001
[57018.735520] cp210x 3-1.2:1.0: cp210x converter detected
[57018.736071] cp210x 3-1.2:1.0: partnum = 0x02
[57018.736529] cp210x 3-1.2:1.0: device does not support event-insertion 
mode
[57018.738780] usb 3-1.2: cp210x converter now attached to ttyUSB0

# sucessfully did a complete mem dump of a device which sends several 
valid 0xEC 0x00
# libdivecomputer/examples/dctool -d "Mares Puck Pro" dump -o 
/tmp/test.bin /dev/ttyUSB0

Opening the I/O stream (serial, /dev/ttyUSB0).
Opening the device (Mares Puck Pro).
Registering the event handler.
Registering the cancellation handler.
Downloading the memory dump.
Event: 
vendor=000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005075636B2050726F000000000000000030342E30302E30300400000031342D30342D31352B8B4443303234000000000000000000000000000000000000000000000000000000
Event: progress 0.00% (0/262144)
Event: progress 0.10% (256/262144)
...
Event: progress 100.00% (262144/262144)



On 22.09.21 13:30, Johan Hovold wrote:
> Some CP2102 do not support event-insertion mode but return no error when
> attempting to enable it.
> 
> This means that any event escape characters in the input stream will not
> be escaped by the device and consequently regular data may be
> interpreted as escape sequences and be removed from the stream by the
> driver.
> 
> The reporter's device has batch number DCL00X etched into it and as
> discovered by the SHA2017 Badge team, counterfeit devices with that
> marking can be detected by sending malformed vendor requests. [1][2]
> 
> Tests confirm that the possibly counterfeit CP2102 returns a single byte
> in response to a malformed two-byte part-number request, while an
> original CP2102 returns two bytes. Assume that every CP2102 that behaves
> this way also does not support event-insertion mode (e.g. cannot report
> parity errors).
> 
> [1] https://mobile.twitter.com/sha2017badge/status/1167902087289532418
> [2] https://hackaday.com/2017/08/14/hands-on-with-the-shacamp-2017-badge/#comment-3903376
> 
> Reported-by: Malte Di Donato <malte@neo-soft.org>
> Fixes: a7207e9835a4 ("USB: serial: cp210x: add support for line-status events")
> Cc: stable@vger.kernel.org	# 5.9
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>   drivers/usb/serial/cp210x.c | 35 +++++++++++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index 66a6ac50a4cd..b98454fe08ea 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -258,6 +258,7 @@ struct cp210x_serial_private {
>   	speed_t			max_speed;
>   	bool			use_actual_rate;
>   	bool			no_flow_control;
> +	bool			no_event_mode;
>   };
>   
>   enum cp210x_event_state {
> @@ -1113,12 +1114,16 @@ static void cp210x_change_speed(struct tty_struct *tty,
>   
>   static void cp210x_enable_event_mode(struct usb_serial_port *port)
>   {
> +	struct cp210x_serial_private *priv = usb_get_serial_data(port->serial);
>   	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
>   	int ret;
>   
>   	if (port_priv->event_mode)
>   		return;
>   
> +	if (priv->no_event_mode)
> +		return;
> +
>   	port_priv->event_state = ES_DATA;
>   	port_priv->event_mode = true;
>   
> @@ -2074,6 +2079,33 @@ static void cp210x_init_max_speed(struct usb_serial *serial)
>   	priv->use_actual_rate = use_actual_rate;
>   }
>   
> +static void cp2102_determine_quirks(struct usb_serial *serial)
> +{
> +	struct cp210x_serial_private *priv = usb_get_serial_data(serial);
> +	u8 *buf;
> +	int ret;
> +
> +	buf = kmalloc(2, GFP_KERNEL);
> +	if (!buf)
> +		return;
> +	/*
> +	 * Some (possibly counterfeit) CP2102 do not support event-insertion
> +	 * mode and respond differently to malformed vendor requests.
> +	 * Specifically, they return one instead of two bytes when sent a
> +	 * two-byte part-number request.
> +	 */
> +	ret = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
> +			CP210X_VENDOR_SPECIFIC, REQTYPE_DEVICE_TO_HOST,
> +			CP210X_GET_PARTNUM, 0, buf, 2, USB_CTRL_GET_TIMEOUT);
> +	if (ret == 1) {
> +		dev_dbg(&serial->interface->dev,
> +				"device does not support event-insertion mode\n");
> +		priv->no_event_mode = true;
> +	}
> +
> +	kfree(buf);
> +}
> +
>   static int cp210x_get_fw_version(struct usb_serial *serial, u16 value)
>   {
>   	struct cp210x_serial_private *priv = usb_get_serial_data(serial);
> @@ -2109,6 +2141,9 @@ static void cp210x_determine_type(struct usb_serial *serial)
>   	}
>   
>   	switch (priv->partnum) {
> +	case CP210X_PARTNUM_CP2102:
> +		cp2102_determine_quirks(serial);
> +		break;
>   	case CP210X_PARTNUM_CP2105:
>   	case CP210X_PARTNUM_CP2108:
>   		cp210x_get_fw_version(serial, CP210X_GET_FW_VER);
>
Johan Hovold Sept. 23, 2021, 8:49 a.m. UTC | #2
On Wed, Sep 22, 2021 at 10:12:48PM +0200, Malte Di Donato wrote:
> Tested-by: Malte Di Donato <malte@neo-soft.org>

Thanks for testing. Now applied.

Johan
diff mbox series

Patch

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 66a6ac50a4cd..b98454fe08ea 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -258,6 +258,7 @@  struct cp210x_serial_private {
 	speed_t			max_speed;
 	bool			use_actual_rate;
 	bool			no_flow_control;
+	bool			no_event_mode;
 };
 
 enum cp210x_event_state {
@@ -1113,12 +1114,16 @@  static void cp210x_change_speed(struct tty_struct *tty,
 
 static void cp210x_enable_event_mode(struct usb_serial_port *port)
 {
+	struct cp210x_serial_private *priv = usb_get_serial_data(port->serial);
 	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
 	int ret;
 
 	if (port_priv->event_mode)
 		return;
 
+	if (priv->no_event_mode)
+		return;
+
 	port_priv->event_state = ES_DATA;
 	port_priv->event_mode = true;
 
@@ -2074,6 +2079,33 @@  static void cp210x_init_max_speed(struct usb_serial *serial)
 	priv->use_actual_rate = use_actual_rate;
 }
 
+static void cp2102_determine_quirks(struct usb_serial *serial)
+{
+	struct cp210x_serial_private *priv = usb_get_serial_data(serial);
+	u8 *buf;
+	int ret;
+
+	buf = kmalloc(2, GFP_KERNEL);
+	if (!buf)
+		return;
+	/*
+	 * Some (possibly counterfeit) CP2102 do not support event-insertion
+	 * mode and respond differently to malformed vendor requests.
+	 * Specifically, they return one instead of two bytes when sent a
+	 * two-byte part-number request.
+	 */
+	ret = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+			CP210X_VENDOR_SPECIFIC, REQTYPE_DEVICE_TO_HOST,
+			CP210X_GET_PARTNUM, 0, buf, 2, USB_CTRL_GET_TIMEOUT);
+	if (ret == 1) {
+		dev_dbg(&serial->interface->dev,
+				"device does not support event-insertion mode\n");
+		priv->no_event_mode = true;
+	}
+
+	kfree(buf);
+}
+
 static int cp210x_get_fw_version(struct usb_serial *serial, u16 value)
 {
 	struct cp210x_serial_private *priv = usb_get_serial_data(serial);
@@ -2109,6 +2141,9 @@  static void cp210x_determine_type(struct usb_serial *serial)
 	}
 
 	switch (priv->partnum) {
+	case CP210X_PARTNUM_CP2102:
+		cp2102_determine_quirks(serial);
+		break;
 	case CP210X_PARTNUM_CP2105:
 	case CP210X_PARTNUM_CP2108:
 		cp210x_get_fw_version(serial, CP210X_GET_FW_VER);