TTY: fix tty_wait_until_sent on 64-bit machines

This series fixes an overflow bug in tty_wait_until_sent, which is currently broken on 64-bit machines for most TTY drivers (with serial-core being the notable exception).

The final patch makes the tty_wait_until_sent timeout a maximum one so that we actually honour the closing-wait timeout.

All but the second (clean-up) and final patch are marked for stable.

These patches are for v4.0 and I suggest Greg takes them all through the TTY tree.

Thanks,
Johan

Johan Hovold (5):
net: irda: fix wait_until_sent poll timeout
TTY: bfin_jtag_comm: remove incorrect wait_until_sent operation
USB: serial: fix infinite wait_until_sent timeout
TTY: fix tty_wait_until_sent on 64-bit machines
TTY: fix tty_wait_until_sent maximum timeout

 drivers/tty/bfin_jtag_comm.c | 13 -------------
 drivers/tty/tty_ioctl.c      | 16 +++++++++++-----
 drivers/usb/serial/generic.c |  5 +++--
 net/irda/ircomm/ircomm_tty.c |  4 +++-
 4 files changed, 17 insertions(+), 21 deletions(-)

2.0.5

In case an infinite timeout (0) is requested, the irda wait_until_sent implementation would use a zero poll timeout rather than the default 200ms.

Note that wait_until_sent is currently never called with a 0-timeout argument due to a bug in tty_wait_until_sent.

---
 net/irda/ircomm/ircomm_tty.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index 40695b9751c1..4efe486baee6 100644
@@ -798,7 +798,9 @@ static void ircomm_tty_wait_until_sent(struct tty_struct *tty, int timeout)
 	orig_jiffies = jiffies;
 
 	/* Set poll time to 200 ms */
-	poll_time = IRDA_MIN(timeout, msecs_to_jiffies(200));
+	poll_time = msecs_to_jiffies(200);
+	if (timeout)
+		poll_time = min_t(unsigned long, timeout, poll_time);
 
 	spin_lock_irqsave(&self->spinlock, flags);
 	while (self->tx_skb && self->tx_skb->len) {

2.0.5

Remove incorrect and redundant wait_until_sent operation, which waits for the driver buffer rather than any hardware buffers to drain, something which is already taken care of by the tty layer (and chars_in_buffer).

---
 drivers/tty/bfin_jtag_comm.c | 13 -------------
 1 file changed, 13 deletions(-)

index d7b198c400c7..ce24182f8514 100644
@@ -210,18 +210,6 @@ bfin_jc_chars_in_buffer(struct tty_struct *tty)
 	return circ_cnt(&bfin_jc_write_buf);
 }
 
-static void
-bfin_jc_wait_until_sent(struct tty_struct *tty, int timeout)
-{
-	unsigned long expire = jiffies + timeout;
-	while (!circ_empty(&bfin_jc_write_buf)) {
-		if (signal_pending(current))
-			break;
-		if (time_after(jiffies, expire))
-			break;
-	}
-}
-
 static const struct tty_operations bfin_jc_ops = {
 	.open            = bfin_jc_open,
 	.close           = bfin_jc_close,

@@ -230,7 +218,6 @@ static const struct tty_operations bfin_jc_ops = {
 	.flush_chars     = bfin_jc_flush_chars,
 	.write_room      = bfin_jc_write_room,
 	.chars_in_buffer = bfin_jc_chars_in_buffer,
-	.wait_until_sent = bfin_jc_wait_until_sent,
 };
 
 static int __init bfin_jc_init(void)

2.0.5

Make sure to handle an infinite timeout (0).

Note that wait_until_sent is currently never called with a 0-timeout argument due to a bug in tty_wait_until_sent.

implementation")
Cc: stable <[email protected]> # v3.10

---
 drivers/usb/serial/generic.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index ccf1df7c4b80..54e170dd3dad 100644
@@ -258,7 +258,8 @@ void usb_serial_generic_wait_until_sent(struct tty_struct *tty, long timeout)
 	 * character or at least one jiffy.
 	 */
 	period = max_t(unsigned long, (10 * HZ / bps), 1);
-	period = min_t(unsigned long, period, timeout);
+	if (timeout)
+		period = min_t(unsigned long, period, timeout);
 
 	dev_dbg(&port->dev, "%s - timeout = %u ms, period = %u ms\n",
 					__func__, jiffies_to_msecs(timeout),

@@ -268,7 +269,7 @@ void usb_serial_generic_wait_until_sent(struct tty_struct *tty, long timeout)
 		schedule_timeout_interruptible(period);
 		if (signal_pending(current))
 			break;
-		if (time_after(jiffies, expire))
+		if (timeout && time_after(jiffies, expire))
 			break;
 	}
 }

2.0.5

Fix overflow bug in tty_wait_until_sent on 64-bit machines, where an infinite timeout (0) would be passed to the underlying tty-driver's wait_until_sent-operation as a negative timeout (-1), causing it to return immediately.

This manifests itself for example as tcdrain() returning immediately, drivers not honouring the drain flags when setting terminal attributes, or even dropped data on close as a requested infinite closing-wait timeout would be ignored.

The first symptom was reported by Asier LLANO who noted that tcdrain() returned prematurely when using the ftdi_sio usb-serial driver.

Fix this by passing 0 rather than MAX_SCHEDULE_TIMEOUT (LONG_MAX) to the underlying tty driver.

Note that the serial-core wait_until_sent-implementation is not affected by this bug due to a lucky chance (comparison to an unsigned maximum timeout), and neither is the cyclades one that had an explicit check for negative timeouts, but all other tty drivers appear to be affected.

---
 drivers/tty/tty_ioctl.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

index a5cf253b2544..89ae23ac9ae6 100644
@@ -217,11 +217,17 @@ void tty_wait_until_sent(struct tty_struct *tty, long timeout)
 #endif
 	if (!timeout)
 		timeout = MAX_SCHEDULE_TIMEOUT;
+
 	if (wait_event_interruptible_timeout(tty->write_wait,
-			!tty_chars_in_buffer(tty), timeout) >= 0) {
-		if (tty->ops->wait_until_sent)
-			tty->ops->wait_until_sent(tty, timeout);
+			!tty_chars_in_buffer(tty), timeout) < 0) {
+		return;
 	}
+
+	if (timeout == MAX_SCHEDULE_TIMEOUT)
+		timeout = 0;
+
+	if (tty->ops->wait_until_sent)
+		tty->ops->wait_until_sent(tty, timeout);
 }
 EXPORT_SYMBOL(tty_wait_until_sent);
 

2.0.5

Wow, that is a nasty bug.

Currently tty_wait_until_sent may take up to twice as long as the requested timeout while waiting for driver and hardware buffers to drain.

Fix this by taking the remaining number of jiffies after waiting for driver buffers to drain into account so that the timeout actually becomes a maximum timeout as it is documented to be.

Note that this specifically implies tighter timings when closing a port as a consequence of actually honouring the port closing-wait setting for drivers relying on tty_wait_until_sent_from_close (e.g. via tty_port_close_start).

---
 drivers/tty/tty_ioctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 89ae23ac9ae6..632fc8152061 100644
@@ -218,10 +218,10 @@ void tty_wait_until_sent(struct tty_struct *tty, long timeout)
 	if (!timeout)
 		timeout = MAX_SCHEDULE_TIMEOUT;
 
-	if (wait_event_interruptible_timeout(tty->write_wait,
-			!tty_chars_in_buffer(tty), timeout) < 0) {
+	timeout = wait_event_interruptible_timeout(tty->write_wait,
+			!tty_chars_in_buffer(tty), timeout);
+	if (timeout <= 0)
 		return;
-	}
 
 	if (timeout == MAX_SCHEDULE_TIMEOUT)
 		timeout = 0;

2.0.5

Nice fixes, all now queued up.