Skip to content

Commit b2ca699

Browse files
jhovoldgregkh
authored andcommitted
USB: serial: fix null-pointer dereferences on disconnect
Make sure serial-driver dtr_rts is called with disc_mutex held after checking the disconnected flag. Due to a bug in the tty layer, dtr_rts may get called after a device has been disconnected and the tty-device unregistered. Some drivers have had individual checks for disconnect to make sure the disconnected interface was not accessed, but this should really be handled in usb-serial core (at least until the long-standing tty-bug has been fixed). Note that the problem has been made more acute with commit 0998d06 ("device-core: Ensure drvdata = NULL when no driver is bound") as the port data is now also NULL when dtr_rts is called resulting in further oopses. Reported-by: Chris Ruehl <chris.ruehl@gtsys.com.hk> Cc: stable <stable@vger.kernel.org> Signed-off-by: Johan Hovold <jhovold@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent cd56527 commit b2ca699

File tree

7 files changed

+50
-59
lines changed

7 files changed

+50
-59
lines changed

drivers/usb/serial/ftdi_sio.c

+9-11
Original file line numberDiff line numberDiff line change
@@ -1886,24 +1886,22 @@ static void ftdi_dtr_rts(struct usb_serial_port *port, int on)
18861886
{
18871887
struct ftdi_private *priv = usb_get_serial_port_data(port);
18881888

1889-
mutex_lock(&port->serial->disc_mutex);
1890-
if (!port->serial->disconnected) {
1891-
/* Disable flow control */
1892-
if (!on && usb_control_msg(port->serial->dev,
1889+
/* Disable flow control */
1890+
if (!on) {
1891+
if (usb_control_msg(port->serial->dev,
18931892
usb_sndctrlpipe(port->serial->dev, 0),
18941893
FTDI_SIO_SET_FLOW_CTRL_REQUEST,
18951894
FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE,
18961895
0, priv->interface, NULL, 0,
18971896
WDR_TIMEOUT) < 0) {
1898-
dev_err(&port->dev, "error from flowcontrol urb\n");
1897+
dev_err(&port->dev, "error from flowcontrol urb\n");
18991898
}
1900-
/* drop RTS and DTR */
1901-
if (on)
1902-
set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
1903-
else
1904-
clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
19051899
}
1906-
mutex_unlock(&port->serial->disc_mutex);
1900+
/* drop RTS and DTR */
1901+
if (on)
1902+
set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
1903+
else
1904+
clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
19071905
}
19081906

19091907
/*

drivers/usb/serial/mct_u232.c

+9-13
Original file line numberDiff line numberDiff line change
@@ -499,19 +499,15 @@ static void mct_u232_dtr_rts(struct usb_serial_port *port, int on)
499499
unsigned int control_state;
500500
struct mct_u232_private *priv = usb_get_serial_port_data(port);
501501

502-
mutex_lock(&port->serial->disc_mutex);
503-
if (!port->serial->disconnected) {
504-
/* drop DTR and RTS */
505-
spin_lock_irq(&priv->lock);
506-
if (on)
507-
priv->control_state |= TIOCM_DTR | TIOCM_RTS;
508-
else
509-
priv->control_state &= ~(TIOCM_DTR | TIOCM_RTS);
510-
control_state = priv->control_state;
511-
spin_unlock_irq(&priv->lock);
512-
mct_u232_set_modem_ctrl(port, control_state);
513-
}
514-
mutex_unlock(&port->serial->disc_mutex);
502+
spin_lock_irq(&priv->lock);
503+
if (on)
504+
priv->control_state |= TIOCM_DTR | TIOCM_RTS;
505+
else
506+
priv->control_state &= ~(TIOCM_DTR | TIOCM_RTS);
507+
control_state = priv->control_state;
508+
spin_unlock_irq(&priv->lock);
509+
510+
mct_u232_set_modem_ctrl(port, control_state);
515511
}
516512

517513
static void mct_u232_close(struct usb_serial_port *port)

drivers/usb/serial/quatech2.c

+8-10
Original file line numberDiff line numberDiff line change
@@ -945,19 +945,17 @@ static void qt2_dtr_rts(struct usb_serial_port *port, int on)
945945
struct usb_device *dev = port->serial->dev;
946946
struct qt2_port_private *port_priv = usb_get_serial_port_data(port);
947947

948-
mutex_lock(&port->serial->disc_mutex);
949-
if (!port->serial->disconnected) {
950-
/* Disable flow control */
951-
if (!on && qt2_setregister(dev, port_priv->device_port,
948+
/* Disable flow control */
949+
if (!on) {
950+
if (qt2_setregister(dev, port_priv->device_port,
952951
UART_MCR, 0) < 0)
953952
dev_warn(&port->dev, "error from flowcontrol urb\n");
954-
/* drop RTS and DTR */
955-
if (on)
956-
update_mctrl(port_priv, TIOCM_DTR | TIOCM_RTS, 0);
957-
else
958-
update_mctrl(port_priv, 0, TIOCM_DTR | TIOCM_RTS);
959953
}
960-
mutex_unlock(&port->serial->disc_mutex);
954+
/* drop RTS and DTR */
955+
if (on)
956+
update_mctrl(port_priv, TIOCM_DTR | TIOCM_RTS, 0);
957+
else
958+
update_mctrl(port_priv, 0, TIOCM_DTR | TIOCM_RTS);
961959
}
962960

963961
static void qt2_update_msr(struct usb_serial_port *port, unsigned char *ch)

drivers/usb/serial/sierra.c

+1-7
Original file line numberDiff line numberDiff line change
@@ -861,19 +861,13 @@ static int sierra_open(struct tty_struct *tty, struct usb_serial_port *port)
861861

862862
static void sierra_dtr_rts(struct usb_serial_port *port, int on)
863863
{
864-
struct usb_serial *serial = port->serial;
865864
struct sierra_port_private *portdata;
866865

867866
portdata = usb_get_serial_port_data(port);
868867
portdata->rts_state = on;
869868
portdata->dtr_state = on;
870869

871-
if (serial->dev) {
872-
mutex_lock(&serial->disc_mutex);
873-
if (!serial->disconnected)
874-
sierra_send_setup(port);
875-
mutex_unlock(&serial->disc_mutex);
876-
}
870+
sierra_send_setup(port);
877871
}
878872

879873
static int sierra_startup(struct usb_serial *serial)

drivers/usb/serial/ssu100.c

+8-11
Original file line numberDiff line numberDiff line change
@@ -506,19 +506,16 @@ static void ssu100_dtr_rts(struct usb_serial_port *port, int on)
506506
{
507507
struct usb_device *dev = port->serial->dev;
508508

509-
mutex_lock(&port->serial->disc_mutex);
510-
if (!port->serial->disconnected) {
511-
/* Disable flow control */
512-
if (!on &&
513-
ssu100_setregister(dev, 0, UART_MCR, 0) < 0)
509+
/* Disable flow control */
510+
if (!on) {
511+
if (ssu100_setregister(dev, 0, UART_MCR, 0) < 0)
514512
dev_err(&port->dev, "error from flowcontrol urb\n");
515-
/* drop RTS and DTR */
516-
if (on)
517-
set_mctrl(dev, TIOCM_DTR | TIOCM_RTS);
518-
else
519-
clear_mctrl(dev, TIOCM_DTR | TIOCM_RTS);
520513
}
521-
mutex_unlock(&port->serial->disc_mutex);
514+
/* drop RTS and DTR */
515+
if (on)
516+
set_mctrl(dev, TIOCM_DTR | TIOCM_RTS);
517+
else
518+
clear_mctrl(dev, TIOCM_DTR | TIOCM_RTS);
522519
}
523520

524521
static void ssu100_update_msr(struct usb_serial_port *port, u8 msr)

drivers/usb/serial/usb-serial.c

+12-2
Original file line numberDiff line numberDiff line change
@@ -694,10 +694,20 @@ static int serial_carrier_raised(struct tty_port *port)
694694
static void serial_dtr_rts(struct tty_port *port, int on)
695695
{
696696
struct usb_serial_port *p = container_of(port, struct usb_serial_port, port);
697-
struct usb_serial_driver *drv = p->serial->type;
697+
struct usb_serial *serial = p->serial;
698+
struct usb_serial_driver *drv = serial->type;
698699

699-
if (drv->dtr_rts)
700+
if (!drv->dtr_rts)
701+
return;
702+
/*
703+
* Work-around bug in the tty-layer which can result in dtr_rts
704+
* being called after a disconnect (and tty_unregister_device
705+
* has returned). Remove once bug has been squashed.
706+
*/
707+
mutex_lock(&serial->disc_mutex);
708+
if (!serial->disconnected)
700709
drv->dtr_rts(p, on);
710+
mutex_unlock(&serial->disc_mutex);
701711
}
702712

703713
static const struct tty_port_operations serial_port_ops = {

drivers/usb/serial/usb_wwan.c

+3-5
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838

3939
void usb_wwan_dtr_rts(struct usb_serial_port *port, int on)
4040
{
41-
struct usb_serial *serial = port->serial;
4241
struct usb_wwan_port_private *portdata;
4342
struct usb_wwan_intf_private *intfdata;
4443

@@ -48,12 +47,11 @@ void usb_wwan_dtr_rts(struct usb_serial_port *port, int on)
4847
return;
4948

5049
portdata = usb_get_serial_port_data(port);
51-
mutex_lock(&serial->disc_mutex);
50+
/* FIXME: locking */
5251
portdata->rts_state = on;
5352
portdata->dtr_state = on;
54-
if (serial->dev)
55-
intfdata->send_setup(port);
56-
mutex_unlock(&serial->disc_mutex);
53+
54+
intfdata->send_setup(port);
5755
}
5856
EXPORT_SYMBOL(usb_wwan_dtr_rts);
5957

0 commit comments

Comments
 (0)