kernel/linux.git/drivers/tty/n_tty.c, branch v3.18.100

n_tty: fix EXTPROC vs ICANON interaction with TIOCINQ (aka FIONREAD)

2018-01-02T19:04:59+00:00

commit 966031f340185eddd05affcf72b740549f056348 upstream. We added support for EXTPROC back in 2010 in commit 26df6d13406d ("tty: Add EXTPROC support for LINEMODE") and the intent was to allow it to override some (all?) ICANON behavior. Quoting from that original commit message: There is a new bit in the termios local flag word, EXTPROC. When this bit is set, several aspects of the terminal driver are disabled. Input line editing, character echo, and mapping of signals are all disabled. This allows the telnetd to turn off these functions when in linemode, but still keep track of what state the user wants the terminal to be in. but the problem turns out that "several aspects of the terminal driver are disabled" is a bit ambiguous, and you can really confuse the n_tty layer by setting EXTPROC and then causing some of the ICANON invariants to no longer be maintained. This fixes at least one such case (TIOCINQ) becoming unhappy because of the confusion over whether ICANON really means ICANON when EXTPROC is set. This basically makes TIOCINQ match the case of read: if EXTPROC is set, we ignore ICANON. Also, make sure to reset the ICANON state ie EXTPROC changes, not just if ICANON changes. Fixes: 26df6d13406d ("tty: Add EXTPROC support for LINEMODE") Reported-by: Tetsuo Handa Reported-by: syzkaller Cc: Jiri Slaby Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman

n_tty: Fix unsafe reference to "other" ldisc

2016-02-15T20:42:32+00:00

[ Upstream commit 6d27a63caad3f13e96cf065d2d96828c2006be6b ] Although n_tty_check_unthrottle() has a valid ldisc reference (since the tty core gets the ldisc ref in tty_read() before calling the line discipline read() method), it does not have a valid ldisc reference to the "other" pty of a pty pair. Since getting an ldisc reference for tty->link essentially open-codes tty_wakeup(), just replace with the equivalent tty_wakeup(). Cc: Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin

tty: fix stall caused by missing memory barrier in drivers/tty/n_tty.c

2015-10-31T13:00:00+00:00

[ Upstream commit e81107d4c6bd098878af9796b24edc8d4a9524fd ] My colleague ran into a program stall on a x86_64 server, where n_tty_read() was waiting for data even if there was data in the buffer in the pty. kernel stack for the stuck process looks like below. #0 [ffff88303d107b58] __schedule at ffffffff815c4b20 #1 [ffff88303d107bd0] schedule at ffffffff815c513e #2 [ffff88303d107bf0] schedule_timeout at ffffffff815c7818 #3 [ffff88303d107ca0] wait_woken at ffffffff81096bd2 #4 [ffff88303d107ce0] n_tty_read at ffffffff8136fa23 #5 [ffff88303d107dd0] tty_read at ffffffff81368013 #6 [ffff88303d107e20] __vfs_read at ffffffff811a3704 #7 [ffff88303d107ec0] vfs_read at ffffffff811a3a57 #8 [ffff88303d107f00] sys_read at ffffffff811a4306 #9 [ffff88303d107f50] entry_SYSCALL_64_fastpath at ffffffff815c86d7 There seems to be two problems causing this issue. First, in drivers/tty/n_tty.c, __receive_buf() stores the data and updates ldata->commit_head using smp_store_release() and then checks the wait queue using waitqueue_active(). However, since there is no memory barrier, __receive_buf() could return without calling wake_up_interactive_poll(), and at the same time, n_tty_read() could start to wait in wait_woken() as in the following chart. __receive_buf() n_tty_read() ------------------------------------------------------------------------ if (waitqueue_active(&tty->read_wait)) /* Memory operations issued after the RELEASE may be completed before the RELEASE operation has completed */ add_wait_queue(&tty->read_wait, &wait); ... if (!input_available_p(tty, 0)) { smp_store_release(&ldata->commit_head, ldata->read_head); ... timeout = wait_woken(&wait, TASK_INTERRUPTIBLE, timeout); ------------------------------------------------------------------------ The second problem is that n_tty_read() also lacks a memory barrier call and could also cause __receive_buf() to return without calling wake_up_interactive_poll(), and n_tty_read() to wait in wait_woken() as in the chart below. __receive_buf() n_tty_read() ------------------------------------------------------------------------ spin_lock_irqsave(&q->lock, flags); /* from add_wait_queue() */ ... if (!input_available_p(tty, 0)) { /* Memory operations issued after the RELEASE may be completed before the RELEASE operation has completed */ smp_store_release(&ldata->commit_head, ldata->read_head); if (waitqueue_active(&tty->read_wait)) __add_wait_queue(q, wait); spin_unlock_irqrestore(&q->lock,flags); /* from add_wait_queue() */ ... timeout = wait_woken(&wait, TASK_INTERRUPTIBLE, timeout); ------------------------------------------------------------------------ There are also other places in drivers/tty/n_tty.c which have similar calls to waitqueue_active(), so instead of adding many memory barrier calls, this patch simply removes the call to waitqueue_active(), leaving just wake_up*() behind. This fixes both problems because, even though the memory access before or after the spinlocks in both wake_up*() and add_wait_queue() can sneak into the critical section, it cannot go past it and the critical section assures that they will be serialized (please see "INTER-CPU ACQUIRING BARRIER EFFECTS" in Documentation/memory-barriers.txt for a better explanation). Moreover, the resulting code is much simpler. Latency measurement using a ping-pong test over a pty doesn't show any visible performance drop. Signed-off-by: Kosuke Tatsukawa Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin

Revert "tty: fix stall caused by missing memory barrier in drivers/tty/n_tty.c"

2015-10-31T12:54:32+00:00

This reverts commit af32cc7bde6304dac92e6a74fe4b2cc8120cb29a. The commit was incorrectly backported and was causing hangs. Reported-by: Corey Wright Signed-off-by: Sasha Levin

tty: fix stall caused by missing memory barrier in drivers/tty/n_tty.c

2015-10-28T02:14:29+00:00

n_tty: Fix auditing support for cannonical mode

2015-07-03T16:34:34+00:00

[ Upstream commit 72586c6061ab8c23ffd9f301ed19782a44ff5f04 ] Commit 32f13521ca68bc624ff6effc77f308a52b038bf0 ("n_tty: Line copy to user buffer in canonical mode") changed cannonical mode copying to use copy_to_user but missed adding the call to the audit framework. Add in the appropriate functions to get audit support. Fixes: 32f13521ca68 ("n_tty: Line copy to user buffer in canonical mode") Reported-by: Miloslav Trmač Signed-off-by: Laura Abbott Reviewed-by: Peter Hurley Cc: stable Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin

n_tty: Fix read buffer overwrite when no newline

2015-04-23T03:26:53+00:00

[ Upstream commit fb5ef9e7da39968fec6d6f37f20a23d23740c75e ] BugLink: http://bugs.launchpad.net/bugs/1381005 In canon mode, the read buffer head will advance over the buffer tail if the input > 4095 bytes without receiving a line termination char. Discard additional input until a line termination is received. Before evaluating for overflow, the 'room' value is normalized for I_PARMRK and 1 byte is reserved for line termination (even in !icanon mode, in case the mode is switched). The following table shows the transform: actual buffer | 'room' value before overflow calc space avail | !I_PARMRK | I_PARMRK -------------------------------------------------- 0 | -1 | -1 1 | 0 | 0 2 | 1 | 0 3 | 2 | 0 4+ | 3 | 1 When !icanon or when icanon and the read buffer contains newlines, normalized 'room' values of -1 and 0 are clamped to 0, and 'overflow' is 0, so read_head is not adjusted and the input i/o loop exits (setting no_room if called from flush_to_ldisc()). No input is discarded since the reader does have input available to read which ensures forward progress. When icanon and the read buffer does not contain newlines and the normalized 'room' value is 0, then overflow and room are reset to 1, so that the i/o loop will process the next input char normally (except for parity errors which are ignored). Thus, erasures, signalling chars, 7-bit mode, etc. will continue to be handled properly. If the input char processed was not a line termination char, then the canon_head index will not have advanced, so the normalized 'room' value will now be -1 and 'overflow' will be set, which indicates the read_head can safely be reset, effectively erasing the last char processed. If the input char processed was a line termination, then the canon_head index will have advanced, so 'overflow' is cleared to 0, the read_head is not reset, and 'room' is cleared to 0, which exits the i/o loop (because the reader now have input available to read which ensures forward progress). Note that it is possible for a line termination to be received, and for the reader to copy the line to the user buffer before the input i/o loop is ready to process the next input char. This is why the i/o loop recomputes the room/overflow state with every input char while handling overflow. Finally, if the input data was processed without receiving a line termination (so that overflow is still set), the pty driver must receive a write wakeup. A pty writer may be waiting to write more data in n_tty_write() but without unthrottling here that wakeup will not arrive, and forward progress will halt. (Normally, the pty writer is woken when the reader reads data out of the buffer and more space become available). Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman (backported from commit fb5ef9e7da39968fec6d6f37f20a23d23740c75e) Signed-off-by: Joseph Salisbury

n_tty: Fix read_buf race condition, increment read_head after pushing data

2015-01-16T14:59:53+00:00

commit 8bfbe2de769afda051c56aba5450391670e769fc upstream. Commit 19e2ad6a09f0c06dbca19c98e5f4584269d913dd ("n_tty: Remove overflow tests from receive_buf() path") moved the increment of read_head into the arguments list of read_buf_addr(). Function calls represent a sequence point in C. Therefore read_head is incremented before the character c is placed in the buffer. Since the circular read buffer is a lock-less design since commit 6d76bd2618535c581f1673047b8341fd291abc67 ("n_tty: Make N_TTY ldisc receive path lockless"), this creates a race condition that leads to communication errors. This patch modifies the code to increment read_head _after_ the data is placed in the buffer and thus fixes the race for non-SMP machines. To fix the problem for SMP machines, memory barriers must be added in a separate patch. Signed-off-by: Christian Riesch Signed-off-by: Greg Kroah-Hartman

tty: Fix pty master poll() after slave closes v2

2014-11-06T20:23:36+00:00

Commit f95499c3030f ("n_tty: Don't wait for buffer work in read() loop") introduces a race window where a pty master can be signalled that the pty slave was closed before all the data that the slave wrote is delivered. Commit f8747d4a466a ("tty: Fix pty master read() after slave closes") fixed the problem in case of n_tty_read, but the problem still exists for n_tty_poll. This can be seen by running 'for ((i=0; i<100;i++));do ./test.py ;done' where test.py is: import os, select, pty (pid, pty_fd) = pty.fork() if pid == 0: os.write(1, 'This string should be received by parent') else: poller = select.epoll() poller.register( pty_fd, select.EPOLLIN ) ready = poller.poll( 1 * 1000 ) for fd, events in ready: if not events & select.EPOLLIN: print 'missed POLLIN event' else: print os.read(fd, 100) poller.close() The string from the slave is missed several times. This patch takes the same approach as the fix for read and special cases this condition for poll. Tested on 3.16. Signed-off-by: Francesco Ruggeri Signed-off-by: Greg Kroah-Hartman

tty: Fix spurious poll() wakeups

2014-09-08T22:55:25+00:00

When the N_TTY line discipline receives data and wakes readers to process the input, polling writers are also mistakenly woken. This is because, although readers and writers are differentiated by different wait queues (tty->read_wait & tty->write_wait), both wait queues are polled together. Thus, reader wakeups without poll flags still cause poll(POLLOUT) to wakeup. For received data, wakeup readers with POLLIN. Preserve the unspecific wakeup in n_tty_packet_mode_flush(), as this action should flag both POLLIN and POLLOUT. Fixes epoll_wait() for edge-triggered EPOLLOUT. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman