Commit 876ae50d authored by Simon Arlott's avatar Simon Arlott Committed by Greg Kroah-Hartman

USB: ftdi_sio: fix race condition in TIOCMIWAIT, and abort of TIOCMIWAIT when the device is removed

There are two issues here, one is that the device is generating
spurious very fast modem status line changes somewhere:

CTS becomes high then low 18µs later:
[121226.924373] ftdi_process_packet: prev rng=0 dsr=10 dcd=0 cts=6
[121226.924378] ftdi_process_packet: status=10 prev=00 diff=10
[121226.924382] ftdi_process_packet: now rng=0 dsr=10 dcd=0 cts=7
(wake_up_interruptible is called)
[121226.924391] ftdi_process_packet: prev rng=0 dsr=10 dcd=0 cts=7
[121226.924394] ftdi_process_packet: status=00 prev=10 diff=10
[121226.924397] ftdi_process_packet: now rng=0 dsr=10 dcd=0 cts=8
(wake_up_interruptible is called)

This wakes up the task in TIOCMIWAIT:
[121226.924405] ftdi_ioctl: 19451 rng=0->0 dsr=10->10 dcd=0->0 cts=6->8
(wait from 20:51:46 returns and observes both changes)

Which then calls TIOCMIWAIT again:
20:51:46.400239 ioctl(3, TIOCMIWAIT, 0x20) = 0
22:11:09.441818 ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS]) = 0
22:11:09.442812 ioctl(3, TIOCMIWAIT, 0x20) = -1 EIO (Input/output error)
(the second wake_up_interruptible takes effect and an I/O error occurs)

The other issue is that TIOCMIWAIT will wait forever (unless the task is
interrupted) if the device is removed.

This change removes the -EIO return that occurs if the counts don't
appear to have changed. Multiple counts may have been processed as
one or the waiting task may have started waiting after recording the
current count.

It adds a bool to indicate that the device has been removed so that
TIOCMIWAIT doesn't wait forever, and wakes up any tasks so that they can
return -EIO.
Signed-off-by: default avatarSimon Arlott <simon@fire.lp0.eu>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent fca5430d
...@@ -76,6 +76,7 @@ struct ftdi_private { ...@@ -76,6 +76,7 @@ struct ftdi_private {
struct async_icount icount; struct async_icount icount;
wait_queue_head_t delta_msr_wait; /* Used for TIOCMIWAIT */ wait_queue_head_t delta_msr_wait; /* Used for TIOCMIWAIT */
char prev_status; /* Used for TIOCMIWAIT */ char prev_status; /* Used for TIOCMIWAIT */
bool dev_gone; /* Used to abort TIOCMIWAIT */
char transmit_empty; /* If transmitter is empty or not */ char transmit_empty; /* If transmitter is empty or not */
struct usb_serial_port *port; struct usb_serial_port *port;
__u16 interface; /* FT2232C, FT2232H or FT4232H port interface __u16 interface; /* FT2232C, FT2232H or FT4232H port interface
...@@ -1681,6 +1682,7 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port) ...@@ -1681,6 +1682,7 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
init_waitqueue_head(&priv->delta_msr_wait); init_waitqueue_head(&priv->delta_msr_wait);
priv->flags = ASYNC_LOW_LATENCY; priv->flags = ASYNC_LOW_LATENCY;
priv->dev_gone = false;
if (quirk && quirk->port_probe) if (quirk && quirk->port_probe)
quirk->port_probe(priv); quirk->port_probe(priv);
...@@ -1839,6 +1841,9 @@ static int ftdi_sio_port_remove(struct usb_serial_port *port) ...@@ -1839,6 +1841,9 @@ static int ftdi_sio_port_remove(struct usb_serial_port *port)
dbg("%s", __func__); dbg("%s", __func__);
priv->dev_gone = true;
wake_up_interruptible_all(&priv->delta_msr_wait);
remove_sysfs_attrs(port); remove_sysfs_attrs(port);
kref_put(&priv->kref, ftdi_sio_priv_release); kref_put(&priv->kref, ftdi_sio_priv_release);
...@@ -2397,15 +2402,12 @@ static int ftdi_ioctl(struct tty_struct *tty, ...@@ -2397,15 +2402,12 @@ static int ftdi_ioctl(struct tty_struct *tty,
*/ */
case TIOCMIWAIT: case TIOCMIWAIT:
cprev = priv->icount; cprev = priv->icount;
while (1) { while (!priv->dev_gone) {
interruptible_sleep_on(&priv->delta_msr_wait); interruptible_sleep_on(&priv->delta_msr_wait);
/* see if a signal did it */ /* see if a signal did it */
if (signal_pending(current)) if (signal_pending(current))
return -ERESTARTSYS; return -ERESTARTSYS;
cnow = priv->icount; cnow = priv->icount;
if (cnow.rng == cprev.rng && cnow.dsr == cprev.dsr &&
cnow.dcd == cprev.dcd && cnow.cts == cprev.cts)
return -EIO; /* no change => error */
if (((arg & TIOCM_RNG) && (cnow.rng != cprev.rng)) || if (((arg & TIOCM_RNG) && (cnow.rng != cprev.rng)) ||
((arg & TIOCM_DSR) && (cnow.dsr != cprev.dsr)) || ((arg & TIOCM_DSR) && (cnow.dsr != cprev.dsr)) ||
((arg & TIOCM_CD) && (cnow.dcd != cprev.dcd)) || ((arg & TIOCM_CD) && (cnow.dcd != cprev.dcd)) ||
...@@ -2414,7 +2416,7 @@ static int ftdi_ioctl(struct tty_struct *tty, ...@@ -2414,7 +2416,7 @@ static int ftdi_ioctl(struct tty_struct *tty,
} }
cprev = cnow; cprev = cnow;
} }
/* not reached */ return -EIO;
break; break;
case TIOCSERGETLSR: case TIOCSERGETLSR:
return get_lsr_info(port, (struct serial_struct __user *)arg); return get_lsr_info(port, (struct serial_struct __user *)arg);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment