[PATCH, RFC 15/30] usbserial: stop using interruptible_sleep_on
From: Arnd Bergmann
Date: Thu Jan 02 2014 - 07:14:26 EST
We really want to kill off interruptible_sleep_on, which is defined
in a way that is always racy. There are four usb-serial drivers using
it to implement their tiocmiwait() functions, which is defined in
a way that always has a race when called from user space.
This patch changes the four drivers in the same way to use an open-coded
prepare_to_wait/finish_wait loop to get rid of the deprecated function
call, but it does not address the fundamental race.
This particular method of implementing it was chosen because it is
least invasive, a better but more invasive alternative would be
to use usb_serial_generic_tiocmiwait, which is something I did not
dare try without access to hardware.
Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
Cc: Johan Hovold <jhovold@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: linux-usb@xxxxxxxxxxxxxxx
---
drivers/usb/serial/ch341.c | 29 ++++++++++++++++--------
drivers/usb/serial/cypress_m8.c | 49 ++++++++++++++++++++++++++---------------
drivers/usb/serial/f81232.c | 29 ++++++++++++++++--------
drivers/usb/serial/pl2303.c | 29 ++++++++++++++++--------
4 files changed, 91 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index c2a4171..f5e0eea 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -508,20 +508,22 @@ static int ch341_tiocmiwait(struct tty_struct *tty, unsigned long arg)
u8 status;
u8 changed;
u8 multi_change = 0;
+ DEFINE_WAIT(wait);
+ int ret;
spin_lock_irqsave(&priv->lock, flags);
prevstatus = priv->line_status;
priv->multi_status_change = 0;
spin_unlock_irqrestore(&priv->lock, flags);
+ ret = 0;
while (!multi_change) {
- interruptible_sleep_on(&port->port.delta_msr_wait);
- /* see if a signal did it */
- if (signal_pending(current))
- return -ERESTARTSYS;
+ prepare_to_wait(&port->port.delta_msr_wait, &wait, TASK_INTERRUPTIBLE);
- if (port->serial->disconnected)
- return -EIO;
+ if (port->serial->disconnected) {
+ ret = -EIO;
+ break;
+ }
spin_lock_irqsave(&priv->lock, flags);
status = priv->line_status;
@@ -534,12 +536,21 @@ static int ch341_tiocmiwait(struct tty_struct *tty, unsigned long arg)
((arg & TIOCM_DSR) && (changed & CH341_BIT_DSR)) ||
((arg & TIOCM_CD) && (changed & CH341_BIT_DCD)) ||
((arg & TIOCM_CTS) && (changed & CH341_BIT_CTS))) {
- return 0;
+ break;
}
+
+ schedule();
+
+ /* see if a signal did it */
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
+ }
+
prevstatus = status;
}
-
- return 0;
+ finish_wait(&port->port.delta_msr_wait, &wait);
+ return ret;
}
static int ch341_tiocmget(struct tty_struct *tty)
diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index 558605d..e05c9c6 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -869,38 +869,51 @@ static int cypress_tiocmiwait(struct tty_struct *tty, unsigned long arg)
{
struct usb_serial_port *port = tty->driver_data;
struct cypress_private *priv = usb_get_serial_port_data(port);
- char diff;
+ char changed;
+ DEFINE_WAIT(wait);
+ int ret;
- for (;;) {
- interruptible_sleep_on(&port->port.delta_msr_wait);
- /* see if a signal did it */
- if (signal_pending(current))
- return -ERESTARTSYS;
+ while (1) {
+ prepare_to_wait(&port->port.delta_msr_wait, &wait, TASK_INTERRUPTIBLE);
- if (port->serial->disconnected)
- return -EIO;
+ if (port->serial->disconnected) {
+ ret = -EIO;
+ break;
+ }
- diff = priv->diff_status;
- if (diff == 0)
- return -EIO; /* no change => error */
+ changed = priv->diff_status;
+ if (changed == 0) {
+ ret = -EIO; /* no change => error */
+ break;
+ }
/* consume all events */
priv->diff_status = 0;
/* return 0 if caller wanted to know about
these bits */
- if (((arg & TIOCM_RNG) && (diff & UART_RI)) ||
- ((arg & TIOCM_DSR) && (diff & UART_DSR)) ||
- ((arg & TIOCM_CD) && (diff & UART_CD)) ||
- ((arg & TIOCM_CTS) && (diff & UART_CTS)))
- return 0;
+ if (((arg & TIOCM_RNG) && (changed & UART_RI)) ||
+ ((arg & TIOCM_DSR) && (changed & UART_DSR)) ||
+ ((arg & TIOCM_CD) && (changed & UART_CD)) ||
+ ((arg & TIOCM_CTS) && (changed & UART_CTS))) {
+ ret = 0;
+ break;
+ }
+
+ schedule();
+
+ /* see if a signal did it */
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
+ }
/* otherwise caller can't care less about what
* happened, and so we continue to wait for
* more events.
*/
}
-
- return 0;
+ finish_wait(&port->port.delta_msr_wait, &wait);
+ return ret;
}
static void cypress_set_termios(struct tty_struct *tty,
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 639a18f..935f2e5 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -249,19 +249,20 @@ static int f81232_tiocmiwait(struct tty_struct *tty, unsigned long arg)
unsigned int prevstatus;
unsigned int status;
unsigned int changed;
+ DEFINE_WAIT(wait);
+ int ret;
spin_lock_irqsave(&priv->lock, flags);
prevstatus = priv->line_status;
spin_unlock_irqrestore(&priv->lock, flags);
while (1) {
- interruptible_sleep_on(&port->port.delta_msr_wait);
- /* see if a signal did it */
- if (signal_pending(current))
- return -ERESTARTSYS;
+ prepare_to_wait(&port->port.delta_msr_wait, &wait, TASK_INTERRUPTIBLE);
- if (port->serial->disconnected)
- return -EIO;
+ if (port->serial->disconnected) {
+ ret = -EIO;
+ break;
+ }
spin_lock_irqsave(&priv->lock, flags);
status = priv->line_status;
@@ -273,12 +274,22 @@ static int f81232_tiocmiwait(struct tty_struct *tty, unsigned long arg)
((arg & TIOCM_DSR) && (changed & UART_DSR)) ||
((arg & TIOCM_CD) && (changed & UART_DCD)) ||
((arg & TIOCM_CTS) && (changed & UART_CTS))) {
- return 0;
+ ret = 0;
+ break;
+ }
+
+ schedule();
+
+ /* see if a signal did it */
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
}
+
prevstatus = status;
}
- /* NOTREACHED */
- return 0;
+ finish_wait(&port->port.delta_msr_wait, &wait);
+ return ret;
}
static int f81232_ioctl(struct tty_struct *tty,
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 1e3318d..e8f30bc 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -594,19 +594,20 @@ static int pl2303_tiocmiwait(struct tty_struct *tty, unsigned long arg)
unsigned int prevstatus;
unsigned int status;
unsigned int changed;
+ DEFINE_WAIT(wait);
+ int ret;
spin_lock_irqsave(&priv->lock, flags);
prevstatus = priv->line_status;
spin_unlock_irqrestore(&priv->lock, flags);
while (1) {
- interruptible_sleep_on(&port->port.delta_msr_wait);
- /* see if a signal did it */
- if (signal_pending(current))
- return -ERESTARTSYS;
+ prepare_to_wait(&port->port.delta_msr_wait, &wait, TASK_INTERRUPTIBLE);
- if (port->serial->disconnected)
- return -EIO;
+ if (port->serial->disconnected) {
+ ret = -EIO;
+ break;
+ }
spin_lock_irqsave(&priv->lock, flags);
status = priv->line_status;
@@ -618,12 +619,22 @@ static int pl2303_tiocmiwait(struct tty_struct *tty, unsigned long arg)
((arg & TIOCM_DSR) && (changed & UART_DSR)) ||
((arg & TIOCM_CD) && (changed & UART_DCD)) ||
((arg & TIOCM_CTS) && (changed & UART_CTS))) {
- return 0;
+ ret = 0;
+ break;
+ }
+
+ schedule();
+
+ /* see if a signal did it */
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
}
+
prevstatus = status;
}
- /* NOTREACHED */
- return 0;
+ finish_wait(&port->port.delta_msr_wait, &wait);
+ return ret;
}
static int pl2303_ioctl(struct tty_struct *tty,
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/