Skip to contentSkip to conversation

USB: serial: fix ioctl hangup race

PATCH New Johan Hovold 1 participant 1 message Sep 3

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

Johan Hovold

author maintainer
#0 lore raw

PATCH 1/1 USB: serial: fix ioctl hangup race

The tty ioctls can race with hangup and end up calling into a tty driver for a device that is already gone or powered down.

Serialise the callbacks using the tty port mutex and add the missing checks to make sure the port has not been hung up before accessing port driver data or hardware.

This specifically avoids dereferencing a NULL-pointer if a device is disconnected during lengthy break signalling.

Note that TIOCGICOUNT and TIOCMIWAIT do not access port driver data or hardware and are therefore not affected by the race.

Fixes f34d7a5b7010 ("tty: The big operations rework")
Reported-by Nick Bowler <[email protected]>
Link https://lore.kernel.org/all/CADyTPExB2kYOOwkO0JqGhKaYVDqO9uS9WCw0J=MCTdVhcGOogA@mail.gmail.com/ open here
Reported-by [email protected]
Closes https://lore.kernel.org/r/[email protected] open here
Cc [email protected] # 2.6.26
Signed-off-by Johan Hovold <[email protected]>

---

Note that the port driver data NULL-deref is unique for USB serial and does specifically not affect cdc-acm.

I'm posting fixes for serial core and USB serial today will take a closer look at the other TTY drivers tomorrow.

Johan

 drivers/usb/serial/generic.c    | 10 +++-
 drivers/usb/serial/usb-serial.c | 94 ++++++++++++++++++++++++++-------
 drivers/usb/serial/xr_serial.c  | 10 ++--
 3 files changed, 86 insertions(+), 28 deletions(-)
drivers/usb/serial/generic.c +9 -1
index 6eaf74930aa3..17272701fab0 100644
@@ -266,6 +266,7 @@ EXPORT_SYMBOL_GPL(usb_serial_generic_chars_in_buffer);
 void usb_serial_generic_wait_until_sent(struct tty_struct *tty, long timeout)
 {
 	struct usb_serial_port *port = tty->driver_data;
+	struct tty_port *tport = &port->port;
 	unsigned int bps;
 	unsigned long period;
 	unsigned long expire;

@@ -285,7 +286,14 @@ void usb_serial_generic_wait_until_sent(struct tty_struct *tty, long timeout)
 					__func__, jiffies_to_msecs(timeout),
 					jiffies_to_msecs(period));
 	expire = jiffies + timeout;
-	while (!port->serial->type->tx_empty(port)) {
+	for (;;) {
+		mutex_lock(&tport->mutex);
+		if (tty_io_error(tty) || port->serial->type->tx_empty(port)) {
+			mutex_unlock(&tport->mutex);
+			break;
+		}
+		mutex_unlock(&tport->mutex);
+
 		schedule_timeout_interruptible(period);
 		if (signal_pending(current))
 			break;
drivers/usb/serial/usb-serial.c +74 -20

178 lines not on this page · raw message

drivers/usb/serial/xr_serial.c +3 -7
index 352c765d8803..c08f4aa14a3d 100644
@@ -850,12 +850,9 @@ static int xr_get_rs485_config(struct tty_struct *tty,
 	struct usb_serial_port *port = tty->driver_data;
 	struct xr_data *data = usb_get_serial_port_data(port);
 
-	down_read(&tty->termios_rwsem);
-	if (copy_to_user(argp, &data->rs485, sizeof(data->rs485))) {
-		up_read(&tty->termios_rwsem);
+	/* core holds port mutex */
+	if (copy_to_user(argp, &data->rs485, sizeof(data->rs485)))
 		return -EFAULT;
-	}
-	up_read(&tty->termios_rwsem);
 
 	return 0;
 }

@@ -871,10 +868,9 @@ static int xr_set_rs485_config(struct tty_struct *tty,
 		return -EFAULT;
 	xr_sanitize_serial_rs485(&rs485);
 
-	down_write(&tty->termios_rwsem);
+	/* core holds termios rwsem and port mutex */
 	data->rs485 = rs485;
 	xr_set_flow_mode(tty, port, NULL);
-	up_write(&tty->termios_rwsem);
 
 	if (copy_to_user(argp, &rs485, sizeof(rs485)))
 		return -EFAULT;
Signature · 1 line
2.55.0