Skip to contentSkip to conversation

pl2303: pl2303_update_line_status data length fix

PATCH In review Horst Schirmeier 2 participants 3 messages Jul 28, 2005 – Aug 8, 2005

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pl2303: pl2303_update_line_status data length fix

Minimum data length must be UART_STATE + 1, as data[UART_STATE] is being accessed for the new line_state. Although PL-2303 hardware is not expected to send data with exactly UART_STATE length, this keeps it on the safe side.

---

drivers/usb/serial/pl2303.c renamed from drivers/usb/serial/pl2303.c.orig +1 -1
@@ -826,7 +826,7 @@ static void pl2303_update_line_status(st
 	struct pl2303_private *priv = usb_get_serial_port_data(port);
 	unsigned long flags;
 	u8 status_idx = UART_STATE;
-	u8 length = UART_STATE;
+	u8 length = UART_STATE + 1;
 
 	if ((le16_to_cpu(port->serial->dev->descriptor.idVendor) == SIEMENS_VENDOR_ID) &&
 	    (le16_to_cpu(port->serial->dev->descriptor.idProduct) == SIEMENS_PRODUCT_ID_X65)) {

--
PGP-Key 0xD40E0E7A

#1 lore raw
Quoted 11 lines · Horst Schirmeier

Minimum data length must be UART_STATE + 1, as data[UART_STATE] is being accessed for the new line_state. Although PL-2303 hardware is not expected to send data with exactly UART_STATE length, this keeps it on the safe side.

---

drivers/usb/serial/pl2303.c static void pl2303_update_line_status(st · line 829

Quoted 5 lines
drivers/usb/serial/pl2303.c renamed from drivers/usb/serial/pl2303.c.orig +1 -1
@@ -826,7 +826,7 @@ static void pl2303_update_line_status(st
 	struct pl2303_private *priv = usb_get_serial_port_data(port);
 	unsigned long flags;
 	u8 status_idx = UART_STATE;
-	u8 length = UART_STATE;
+	u8 length = UART_STATE + 1;

"safe side" yes, but this will just prevent any line changes from going back to the user, right?

Hm, how is this working at all, it looks like we overflow the buffer...

Have you tested this change?

thanks,

greg k-h

#2 lore raw
Quoted 9 lines · Greg KH
Quoted 8 lines · Horst Schirmeier

Minimum data length must be UART_STATE + 1, as data[UART_STATE] is being accessed for the new line_state. Although PL-2303 hardware is not expected to send data with exactly UART_STATE length, this keeps it on the safe side.

---

Quoted 11 lines

drivers/usb/serial/pl2303.c static void pl2303_update_line_status(st · line 829

Quoted 8 lines
drivers/usb/serial/pl2303.c renamed from drivers/usb/serial/pl2303.c.orig +1 -1
@@ -826,7 +826,7 @@ static void pl2303_update_line_status(st
 	struct pl2303_private *priv = usb_get_serial_port_data(port);
 	unsigned long flags;
 	u8 status_idx = UART_STATE;
-	u8 length = UART_STATE;
+	u8 length = UART_STATE + 1;

"safe side" yes, but this will just prevent any line changes from going back to the user, right?

IMHO not, no. The PL-2303 interrupt IN endpoint has a 10 bytes FIFO and (as far as I've observed from the type_1 model) always sends packets with exactly this size. The UART_STATE is located at offset 0x08, therefore the minimum packet length we need is 0x09 (UART_STATE + 1).

Here are the two different byte sequences from that endpoint I picked up; the first with a terminal on the serial end, the second without one (both times using the patched pl2303 module with debug=1):

a1 20 00 00 00 00 02 00 00 00
a1 20 00 00 00 00 02 00 82 00

Note the UART_STATE byte with the DSR and CTS bits set in the second case.

Quoted 1 line

Hm, how is this working at all, it looks like we overflow the buffer...

In the unlikely case that some weird hardware sends a packet with exactly UART_STATE (8) bytes length, yes, we do. Normal PL-2303 hardware _should_ cause no trouble, as it always sends 10 bytes (at least my hardware does), therefore my side note "this keeps it on the safe side".

Quoted 1 line

Have you tested this change?

Yes, I have.

Quoted 3 lines

thanks,

greg k-h

Kind regards,
Horst

Signature · 1 line
PGP-Key 0xD40E0E7A