kernel/linux.git/drivers/tty/pty.c, branch v4.4.239

tty: Fix data race in tty_insert_flip_string_fixed_flag

2018-08-06T14:24:36+00:00

[ Upstream commit b6da31b2c07c46f2dcad1d86caa835227a16d9ff ] Unlike normal serials, in pty layer, there is no guarantee that multiple threads don't insert input characters at the same time. If it is happened, tty_insert_flip_string_fixed_flag can be executed concurrently. This can lead slab out-of-bounds write in tty_insert_flip_string_fixed_flag. Call sequences are as follows. CPU0 CPU1 n_tty_ioctl_helper n_tty_ioctl_helper __start_tty tty_send_xchar tty_wakeup pty_write n_hdlc_tty_wakeup tty_insert_flip_string n_hdlc_send_frames tty_insert_flip_string_fixed_flag pty_write tty_insert_flip_string tty_insert_flip_string_fixed_flag To fix the race, acquire port->lock in pty_write() before it inserts input characters to tty buffer. It prevents multiple threads from inserting input characters concurrently. The crash log is as follows: BUG: KASAN: slab-out-of-bounds in tty_insert_flip_string_fixed_flag+0xb5/ 0x130 drivers/tty/tty_buffer.c:316 at addr ffff880114fcc121 Write of size 1792 by task syz-executor0/30017 CPU: 1 PID: 30017 Comm: syz-executor0 Not tainted 4.8.0 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 0000000000000000 ffff88011638f888 ffffffff81694cc3 ffff88007d802140 ffff880114fcb300 ffff880114fcc300 ffff880114fcb300 ffff88011638f8b0 ffffffff8130075c ffff88011638f940 ffff88007d802140 ffff880194fcc121 Call Trace: __dump_stack lib/dump_stack.c:15 [inline] dump_stack+0xb3/0x110 lib/dump_stack.c:51 kasan_object_err+0x1c/0x70 mm/kasan/report.c:156 print_address_description mm/kasan/report.c:194 [inline] kasan_report_error+0x1f7/0x4e0 mm/kasan/report.c:283 kasan_report+0x36/0x40 mm/kasan/report.c:303 check_memory_region_inline mm/kasan/kasan.c:292 [inline] check_memory_region+0x13e/0x1a0 mm/kasan/kasan.c:299 memcpy+0x37/0x50 mm/kasan/kasan.c:335 tty_insert_flip_string_fixed_flag+0xb5/0x130 drivers/tty/tty_buffer.c:316 tty_insert_flip_string include/linux/tty_flip.h:35 [inline] pty_write+0x7f/0xc0 drivers/tty/pty.c:115 n_hdlc_send_frames+0x1d4/0x3b0 drivers/tty/n_hdlc.c:419 n_hdlc_tty_wakeup+0x73/0xa0 drivers/tty/n_hdlc.c:496 tty_wakeup+0x92/0xb0 drivers/tty/tty_io.c:601 __start_tty.part.26+0x66/0x70 drivers/tty/tty_io.c:1018 __start_tty+0x34/0x40 drivers/tty/tty_io.c:1013 n_tty_ioctl_helper+0x146/0x1e0 drivers/tty/tty_ioctl.c:1138 n_hdlc_tty_ioctl+0xb3/0x2b0 drivers/tty/n_hdlc.c:794 tty_ioctl+0xa85/0x16d0 drivers/tty/tty_io.c:2992 vfs_ioctl fs/ioctl.c:43 [inline] do_vfs_ioctl+0x13e/0xba0 fs/ioctl.c:679 SYSC_ioctl fs/ioctl.c:694 [inline] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685 entry_SYSCALL_64_fastpath+0x1f/0xbd Signed-off-by: DaeRyong Jeong Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin Signed-off-by: Greg Kroah-Hartman

tty: pty: Fix ldisc flush after userspace become aware of the data already

2017-05-20T12:27:02+00:00

commit 77dae6134440420bac334581a3ccee94cee1c054 upstream. While using emacs, cat or others' commands in konsole with recent kernels, I have met many times that CTRL-C freeze konsole. After konsole freeze I can't type anything, then I have to open a new one, it is very annoying. See bug report: https://bugs.kde.org/show_bug.cgi?id=175283 The platform in that bug report is Solaris, but now the pty in linux has the same problem or the same behavior as Solaris :) It has high possibility to trigger the problem follow steps below: Note: In my test, BigFile is a text file whose size is bigger than 1G 1:open konsole 1:cat BigFile 2:CTRL-C After some digging, I find out the reason is that commit 1d1d14da12e7 ("pty: Fix buffer flush deadlock") changes the behavior of pty_flush_buffer. Thread A Thread B -------- -------- 1:n_tty_poll return POLLIN 2:CTRL-C trigger pty_flush_buffer tty_buffer_flush n_tty_flush_buffer 3:attempt to check count of chars: ioctl(fd, TIOCINQ, &available) available is equal to 0 4:read(fd, buffer, avaiable) return 0 5:konsole close fd Yes, I know we could use the same patch included in the BUG report as a workaround for linux platform too. But I think the data in ldisc is belong to application of another side, we shouldn't clear it when we want to flush write buffer of this side in pty_flush_buffer. So I think it is better to disable ldisc flush in pty_flush_buffer, because its new hehavior bring no benefit except that it mess up the behavior between POLLIN, and TIOCINQ or FIONREAD. Also I find no flush_buffer function in others' tty driver has the same behavior as current pty_flush_buffer. Fixes: 1d1d14da12e7 ("pty: Fix buffer flush deadlock") Signed-off-by: Wang YanQing Signed-off-by: Greg Kroah-Hartman

devpts: clean up interface to pty drivers

2016-08-16T07:30:49+00:00

commit 67245ff332064c01b760afa7a384ccda024bfd24 upstream. This gets rid of the horrible notion of having that struct inode *ptmx_inode be the linchpin of the interface between the pty code and devpts. By de-emphasizing the ptmx inode, a lot of things actually get cleaner, and we will have a much saner way forward. In particular, this will allow us to associate with any particular devpts instance at open-time, and not be artificially tied to one particular ptmx inode. The patch itself is actually fairly straightforward, and apart from some locking and return path cleanups it's pretty mechanical: - the interfaces that devpts exposes all take "struct pts_fs_info *" instead of "struct inode *ptmx_inode" now. NOTE! The "struct pts_fs_info" thing is a completely opaque structure as far as the pty driver is concerned: it's still declared entirely internally to devpts. So the pty code can't actually access it in any way, just pass it as a "cookie" to the devpts code. - the "look up the pts fs info" is now a single clear operation, that also does the reference count increment on the pts superblock. So "devpts_add/del_ref()" is gone, and replaced by a "lookup and get ref" operation (devpts_get_ref(inode)), along with a "put ref" op (devpts_put_ref()). - the pty master "tty->driver_data" field now contains the pts_fs_info, not the ptmx inode. - because we don't care about the ptmx inode any more as some kind of base index, the ref counting can now drop the inode games - it just gets the ref on the superblock. - the pts_fs_info now has a back-pointer to the super_block. That's so that we can easily look up the information we actually need. Although quite often, the pts fs info was actually all we wanted, and not having to look it up based on some magical inode makes things more straightforward. In particular, now that "devpts_get_ref(inode)" operation should really be the *only* place we need to look up what devpts instance we're associated with, and we do it exactly once, at ptmx_open() time. The other side of this is that one ptmx node could now be associated with multiple different devpts instances - you could have a single /dev/ptmx node, and then have multiple mount namespaces with their own instances of devpts mounted on /dev/pts/. And that's all perfectly sane in a model where we just look up the pts instance at open time. This will eventually allow us to get rid of our odd single-vs-multiple pts instance model, but this patch in itself changes no semantics, only an internal binding model. Cc: Eric Biederman Cc: Peter Anvin Cc: Andy Lutomirski Cc: Al Viro Cc: Peter Hurley Cc: Serge Hallyn Cc: Willy Tarreau Cc: Aurelien Jarno Cc: Alan Cox Cc: Jann Horn Cc: Greg KH Cc: Jiri Slaby Cc: Florian Weimer Signed-off-by: Linus Torvalds Cc: Francesco Ruggeri Cc: "Herton R. Krzesinski" Signed-off-by: Greg Kroah-Hartman

Fix OpenSSH pty regression on close

2016-06-01T19:15:52+00:00

commit 0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa upstream. OpenSSH expects the (non-blocking) read() of pty master to return EAGAIN only if it has received all of the slave-side output after it has received SIGCHLD. This used to work on pre-3.12 kernels. This fix effectively forces non-blocking read() and poll() to block for parallel i/o to complete for all ttys. It also unwinds these changes: 1) f8747d4a466ab2cafe56112c51b3379f9fdb7a12 tty: Fix pty master read() after slave closes 2) 52bce7f8d4fc633c9a9d0646eef58ba6ae9a3b73 pty, n_tty: Simplify input processing on final close 3) 1a48632ffed61352a7810ce089dc5a8bcd505a60 pty: Fix input race when closing Inspired by analysis and patch from Marc Aurele La France Reported-by: Volth Reported-by: Marc Aurele La France BugLink: https://bugzilla.mindrot.org/show_bug.cgi?id=52 BugLink: https://bugzilla.mindrot.org/show_bug.cgi?id=2492 Signed-off-by: Brian Bloniarz Reviewed-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

pty: make sure super_block is still valid in final /dev/tty close

2016-02-25T20:01:14+00:00

commit 1f55c718c290616889c04946864a13ef30f64929 upstream. Considering current pty code and multiple devpts instances, it's possible to umount a devpts file system while a program still has /dev/tty opened pointing to a previosuly closed pty pair in that instance. In the case all ptmx and pts/N files are closed, umount can be done. If the program closes /dev/tty after umount is done, devpts_kill_index will use now an invalid super_block, which was already destroyed in the umount operation after running ->kill_sb. This is another "use after free" type of issue, but now related to the allocated super_block instance. To avoid the problem (warning at ida_remove and potential crashes) for this specific case, I added two functions in devpts which grabs additional references to the super_block, which pty code now uses so it makes sure the super block structure is still valid until pty shutdown is done. I also moved the additional inode references to the same functions, which also covered similar case with inode being freed before /dev/tty final close/shutdown. Signed-off-by: Herton R. Krzesinski Reviewed-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

pty: fix possible use after free of tty->driver_data

2016-02-25T20:01:14+00:00

commit 2831c89f42dcde440cfdccb9fee9f42d54bbc1ef upstream. This change fixes a bug for a corner case where we have the the last release from a pty master/slave coming from a previously opened /dev/tty file. When this happens, the tty->driver_data can be stale, due to all ptmx or pts/N files having already been closed before (and thus the inode related to these files, which tty->driver_data points to, being already freed/destroyed). The fix here is to keep a reference on the opened master ptmx inode. We maintain the inode referenced until the final pty_unix98_shutdown, and only pass this inode to devpts_kill_index. Signed-off-by: Herton R. Krzesinski Reviewed-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

drivers/tty: make pty.c slightly more explicitly non-modular

2015-10-04T16:27:56+00:00

The Kconfig currently controlling compilation of this code is: drivers/tty/Kconfig:config LEGACY_PTYS drivers/tty/Kconfig: bool "Legacy (BSD) PTY support" ...and: drivers/tty/Kconfig:config UNIX98_PTYS drivers/tty/Kconfig: bool "Unix98 PTY support" if EXPERT combined with this: obj-$(CONFIG_LEGACY_PTYS) += pty.o obj-$(CONFIG_UNIX98_PTYS) += pty.o ...meaning that it currently is not being built as a module by anyone. Lets remove the traces of modularity we can so that when reading the driver there is less doubt it is builtin-only. Since module_init translates to device_initcall in the non-modular case, the init ordering remains unchanged with this commit. We don't delete the module.h include since other parts of the file are using content from there. Cc: Greg Kroah-Hartman Cc: Jiri Slaby Signed-off-by: Paul Gortmaker Signed-off-by: Greg Kroah-Hartman

pty: Add debug message for ptmx open

2015-07-24T01:37:32+00:00

Opens of /dev/ptmx don't use tty_open() so debug messages are not printed for those opens; print a debug message with the open count (which must always be 1) if TTY_DEBUG_HANGUP is defined. NB: Each tty core source file undefs support for debug messages. The relevant source file must be patched/edited to enable these messages. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

pty: Fix input race when closing

2015-05-10T17:26:37+00:00

A read() from a pty master may mistakenly indicate EOF (errno == -EIO) after the pty slave has closed, even though input data remains to be read. For example, pty slave | input worker | pty master | | | | n_tty_read() pty_write() | | input avail? no add data | | sleep schedule worker --->| | . |---> flush_to_ldisc() | . pty_close() | fill read buffer | . wait for worker | wakeup reader --->| . | read buffer full? |---> input avail ? yes |<--- yes - exit worker | copy 4096 bytes to user TTY_OTHER_CLOSED <---| |<--- kick worker | | **** New read() before worker starts **** | | n_tty_read() | | input avail? no | | TTY_OTHER_CLOSED? yes | | return -EIO Several conditions are required to trigger this race: 1. the ldisc read buffer must become full so the input worker exits 2. the read() count parameter must be >= 4096 so the ldisc read buffer is empty 3. the subsequent read() occurs before the kicked worker has processed more input However, the underlying cause of the race is that data is pipelined, while tty state is not; ie., data already written by the pty slave end is not yet visible to the pty master end, but state changes by the pty slave end are visible to the pty master end immediately. Pipeline the TTY_OTHER_CLOSED state through input worker to the reader. 1. Introduce TTY_OTHER_DONE which is set by the input worker when TTY_OTHER_CLOSED is set and either the input buffers are flushed or input processing has completed. Readers/polls are woken when TTY_OTHER_DONE is set. 2. Reader/poll checks TTY_OTHER_DONE instead of TTY_OTHER_CLOSED. 3. A new input worker is started from pty_close() after setting TTY_OTHER_CLOSED, which ensures the TTY_OTHER_DONE state will be set if the last input worker is already finished (or just about to exit). Remove tty_flush_to_ldisc(); no in-tree callers. Fixes: 52bce7f8d4fc ("pty, n_tty: Simplify input processing on final close") Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96311 BugLink: http://bugs.launchpad.net/bugs/1429756 Cc: # 3.19+ Reported-by: Andy Whitcroft Reported-by: H.J. Lu Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

n_tty: Fix signal handling flushes

2015-02-02T18:11:27+00:00

BRKINT and ISIG requires input and output flush when a signal char is received. However, the order of operations is significant since parallel i/o may be ongoing. Merge the signal handling for BRKINT with ISIG handling. Process the signal first. This ensures any ongoing i/o is aborted; without this, a waiting writer may continue writing after the flush occurs and after the signal char has been echoed. Write lock the termios_rwsem, which excludes parallel writers from pushing new i/o until after the output buffers are flushed; claiming the write lock is necessary anyway to exclude parallel readers while the read buffer is flushed. Subclass the termios_rwsem for ptys since the slave pty performing the flush may appear to reorder the termios_rwsem->tty buffer lock lock order; adding annotation clarifies that slave tty_buffer lock-> slave termios_rwsem -> master tty_buffer lock is a valid lock order. Flush the echo buffer. In this context, the echo buffer is 'output'. Otherwise, the output will appear discontinuous because the output buffer was cleared which contains older output than the echo buffer. Open-code the read buffer flush since the input worker does not need kicking (this is the input worker). Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman