Skip to contentSkip to conversation

USB: serial: digi_acceleport: fix write buffer corruption

PATCH In review Johan Hovold 2 participants 2 messages Jun 23 – Jun 25

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

Johan Hovold

author maintainer
#0 lore raw

PATCH 1/1 USB: serial: digi_acceleport: fix write buffer corruption

The digi_write_inb_command() is supposed to wait for the write urb to become available or return an error, but instead it updates the transfer buffer and tries to resubmit the urb on timeout.

To make things worse, for commands like break control where no timeout is used, the driver would corrupt the urb immediately due to a broken jiffies comparison (on 32-bit machines this takes five minutes of uptime to trigger due to INITIAL_JIFFIES).

Fix this by adding the missing return on timeout and waiting indefinitely when no timeout has been specified as intended.

This issue was (sort of) flagged by Sashiko when reviewing an unrelated change to the driver.

Link https://sashiko.dev/#/patchset/20260610132232.356139-1-johan%40kernel.org?part=11
Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/digi_acceleport.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/usb/serial/digi_acceleport.c +10 -5
index 88c61030982a..fa5c3539f806 100644
@@ -429,20 +429,22 @@ static int digi_write_inb_command(struct usb_serial_port *port,
 	int len;
 	struct digi_port *priv = usb_get_serial_port_data(port);
 	unsigned char *data = port->write_urb->transfer_buffer;
+	unsigned long expire;
 	unsigned long flags;
 
 	dev_dbg(&port->dev, "digi_write_inb_command: TOP: port=%d, count=%d\n",
 		priv->dp_port_num, count);
 
 	if (timeout)
-		timeout += jiffies;
-	else
-		timeout = ULONG_MAX;
+		expire = jiffies + timeout;
 
 	spin_lock_irqsave(&priv->dp_port_lock, flags);
 	while (count > 0 && ret == 0) {
-		while (priv->dp_write_urb_in_use &&
-		       time_before(jiffies, timeout)) {
+		while (priv->dp_write_urb_in_use) {
+			if (timeout && time_after(jiffies, expire)) {
+				ret = -ETIMEDOUT;
+				break;
+			}
 			cond_wait_interruptible_timeout_irqrestore(
 				&priv->write_wait, DIGI_RETRY_TIMEOUT,
 				&priv->dp_port_lock, flags);

@@ -451,6 +453,9 @@ static int digi_write_inb_command(struct usb_serial_port *port,
 			spin_lock_irqsave(&priv->dp_port_lock, flags);
 		}
 
+		if (ret)
+			break;
+
 		/* len must be a multiple of 4 and small enough to */
 		/* guarantee the write will send buffered data first, */
 		/* so commands are in order with data and not split */
Signature · 1 line
2.53.0
maintainer Reviewed-by on v1 1/1
#1 lore raw
Quoted 12 lines · Johan Hovold

The digi_write_inb_command() is supposed to wait for the write urb to become available or return an error, but instead it updates the transfer buffer and tries to resubmit the urb on timeout.

To make things worse, for commands like break control where no timeout is used, the driver would corrupt the urb immediately due to a broken jiffies comparison (on 32-bit machines this takes five minutes of uptime to trigger due to INITIAL_JIFFIES).

Fix this by adding the missing return on timeout and waiting indefinitely when no timeout has been specified as intended.

Quoted 7 lines

This issue was (sort of) flagged by Sashiko when reviewing an unrelated change to the driver.

Link https://sashiko.dev/#/patchset/20260610132232.356139-1-johan%40kernel.org?part=11
Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
Reviewed-by Greg Kroah-Hartman <[email protected]>