Possible bug in the cp210x kernel driver?

Hi Johan,

Sorry about the html crap.
(and double excuse that I've totally forgotten to change my webmail account surname since wedding ;)

Placed the patch code into my running debian kernel version 5.10.0-8-amd64 and put the two invocations to the cp210x_init_max_speed function (cp210x_get_fw_version not existent in my kernel source file).

Here's the dmesg with the patched driver:

[19089.502913] usbcore: registered new interface driver cp210x
[19089.502951] usbserial: USB Serial support registered for cp210x
[19096.084910] usb 3-1.2: new full-speed USB device number 9 using ehci-pci
[19096.224973] usb 3-1.2: New USB device found, idVendor=10c4, idProduct=ea60, bcdDevice= 1.00
[19096.225034] usb 3-1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[19096.225037] usb 3-1.2: Product: CP2102 USB to UART Bridge Controller
[19096.225040] usb 3-1.2: Manufacturer: Silicon Labs
[19096.225043] usb 3-1.2: SerialNumber: 0001
[19096.233460] cp210x 3-1.2:1.0: cp210x converter detected
[19096.233939] cp210x 3-1.2:1.0: cp210x_init_max_speed - type = 0x02
[19096.234458] cp210x 3-1.2:1.0: failed to get vendor val 0x370b size 8: 1

[19096.236568] usb 3-1.2: cp210x converter now attached to ttyUSB0

I also compiled the cp2102-det and run it against the chip:
09: Got 128 bytes: FF F0 FF FA 01 00 60 E3 16 00 FF F0 FF FA 01 00 60 E3 16 00 FF EC FF F8 01 00 80 4F 12 00 FF E6 FF F6 01 00 00 10 0E 00 FF D6 FF F0 01 00 00 CA 08 00 FF D0 FF EE 01 00 20 A1 07 00 FF CC FF EC 01 00 00 08 07 00 FF A2 FF DC 01 00 00 E8 03 00 FF A0 FF DC 01 00 90 D0 03 00 FF 98 FF D9 01 00 00 84 03 00 FF 64 FF C5 01 00 00 58 02 00 FF 44 FF B9 01 00 00 F4 01 00 FF 30 FF B2 01 00 00 C2 0A: Got 1 bytes: FF
0B: Got 1 bytes: 02
Done.

Hope this helps?

As I look on the "SerialNumber: 0001" in dmesg it may be possible that I've got a cloned chip on the chinesium usb/ttl converter...

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 <[email protected]>

Fixes a7207e9835a4 ("USB: serial: cp210x: add support for line-status events")

Cc [email protected] # 5.9

Signed-off-by Johan Hovold <[email protected]>

---
  drivers/usb/serial/cp210x.c | 35 +++++++++++++++++++++++++++++++++++
  1 file changed, 35 insertions(+)

drivers/usb/serial/cp210x.c +35 -0

index 66a6ac50a4cd..b98454fe08ea 100644
@@ -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);

Tested-by Malte Di Donato <[email protected]>

Heh, good that you noticed in time so I can give you proper credit in the commit message. ;)

Unfortunately your mails won't be seen by anyone else one the list unless you can disable that html.

I'll reply to this mail with a fix that fixes the dropped character issue by disabling event-insertion mode (and thus parity reporting) for devices such as yours.

I've verified it here as well as I could, but could you give it a spin with your device as well? If you want you can reply to the patch mail with a Tested-by tag and I'll add that to the commit message as well.

Johan

Quoted 2 lines · Johan Hovold

Unfortunately your mails won't be seen by anyone else one the list unless you can disable that html.

finally it seems that I've got fixed that by now. Not so easy with "limited everything" on vacation :/

Quoted 7 lines

I'll reply to this mail with a fix that fixes the dropped character issue by disabling event-insertion mode (and thus parity reporting) for devices such as yours.

I've verified it here as well as I could, but could you give it a spin with your device as well? If you want you can reply to the patch mail with a Tested-by tag and I'll add that to the commit message as well.

Successfully tested the patch in debian kernel 5.10.0-8-amd64!