From 925bb1ce47f429f69aad35876df7ecd8c53deb7e Mon Sep 17 00:00:00 2001 From: Vegard Nossum Date: Thu, 11 May 2017 12:18:52 +0200 Subject: tty: fix port buffer locking tty_insert_flip_string_fixed_flag() is racy against itself when called from the ioctl(TCXONC, TCION/TCIOFF) path [1] and the flush_to_ldisc() workqueue path [2]. The problem is that port->buf.tail->used is modified without consistent locking; the ioctl path takes tty->atomic_write_lock, whereas the workqueue path takes ldata->output_lock. We cannot simply take ldata->output_lock, since that is specific to the N_TTY line discipline. It might seem natural to try to take port->buf.lock inside tty_insert_flip_string_fixed_flag() and friends (where port->buf is actually used/modified), but this creates problems for flush_to_ldisc() which takes it before grabbing tty->ldisc_sem, o_tty->termios_rwsem, and ldata->output_lock. Therefore, the simplest solution for now seems to be to take tty->atomic_write_lock inside tty_port_default_receive_buf(). This lock is also used in the write path [3] with a consistent ordering. [1]: Call Trace: tty_insert_flip_string_fixed_flag pty_write tty_send_xchar // down_read(&o_tty->termios_rwsem) // mutex_lock(&tty->atomic_write_lock) n_tty_ioctl_helper n_tty_ioctl tty_ioctl // down_read(&tty->ldisc_sem) do_vfs_ioctl SyS_ioctl [2]: Workqueue: events_unbound flush_to_ldisc Call Trace: tty_insert_flip_string_fixed_flag pty_write tty_put_char __process_echoes commit_echoes // mutex_lock(&ldata->output_lock) n_tty_receive_buf_common n_tty_receive_buf2 tty_ldisc_receive_buf // down_read(&o_tty->termios_rwsem) tty_port_default_receive_buf // down_read(&tty->ldisc_sem) flush_to_ldisc // mutex_lock(&port->buf.lock) process_one_work [3]: Call Trace: tty_insert_flip_string_fixed_flag pty_write n_tty_write // mutex_lock(&ldata->output_lock) // down_read(&tty->termios_rwsem) do_tty_write (inline) // mutex_lock(&tty->atomic_write_lock) tty_write // down_read(&tty->ldisc_sem) __vfs_write vfs_write SyS_write The bug can result in about a dozen different crashes depending on what exactly gets corrupted when port->buf.tail->used points outside the buffer. The patch passes my LOCKDEP/PROVE_LOCKING testing but more testing is always welcome. Found using syzkaller. Cc: Signed-off-by: Vegard Nossum Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_port.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/tty/tty_port.c') diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index 0c880f17d27e..88dac3b79369 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c @@ -33,7 +33,9 @@ static int tty_port_default_receive_buf(struct tty_port *port, if (!disc) return 0; + mutex_lock(&tty->atomic_write_lock); ret = tty_ldisc_receive_buf(disc, p, (char *)f, count); + mutex_unlock(&tty->atomic_write_lock); tty_ldisc_deref(disc); -- cgit