From a05a56507786908fdc5b9751323fc886fd19be0f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ilpo=20J=C3=A4rvinen?= Date: Thu, 11 May 2023 15:32:43 +0300 Subject: [PATCH] tty: Prevent writing chars during tcsetattr TCSADRAIN/FLUSH MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit If userspace races tcsetattr() with a write, the drained condition might not be guaranteed by the kernel. There is a race window after checking Tx is empty before tty_set_termios() takes termios_rwsem for write. During that race window, more characters can be queued by a racing writer. Any ongoing transmission might produce garbage during HW's ->set_termios() call. The intent of TCSADRAIN/FLUSH seems to be preventing such a character corruption. If those flags are set, take tty's write lock to stop any writer before performing the lower layer Tx empty check and wait for the pending characters to be sent (if any). The initial wait for all-writers-done must be placed outside of tty's write lock to avoid deadlock which makes it impossible to use tty_wait_until_sent(). The write lock is retried if a racing write is detected. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Signed-off-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230317113318.31327-2-ilpo.jarvinen@linux.intel.com Signed-off-by: Greg Kroah-Hartman (cherry picked from commit 094fb49a2d0d6827c86d2e0840873e6db0c491d2) Signed-off-by: Ilpo Järvinen Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_io.c | 4 ++-- drivers/tty/tty_ioctl.c | 45 ++++++++++++++++++++++++++++++----------- include/linux/tty.h | 2 ++ 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 37c46c65a65f4..bb14bd4f6e55d 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -876,13 +876,13 @@ static ssize_t tty_read(struct file *file, char __user *buf, size_t count, return i; } -static void tty_write_unlock(struct tty_struct *tty) +void tty_write_unlock(struct tty_struct *tty) { mutex_unlock(&tty->atomic_write_lock); wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT); } -static int tty_write_lock(struct tty_struct *tty, int ndelay) +int tty_write_lock(struct tty_struct *tty, int ndelay) { if (!mutex_trylock(&tty->atomic_write_lock)) { if (ndelay) diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c index 9245fffdbceb1..5d833f0b412d5 100644 --- a/drivers/tty/tty_ioctl.c +++ b/drivers/tty/tty_ioctl.c @@ -397,21 +397,42 @@ static int set_termios(struct tty_struct *tty, void __user *arg, int opt) tmp_termios.c_ispeed = tty_termios_input_baud_rate(&tmp_termios); tmp_termios.c_ospeed = tty_termios_baud_rate(&tmp_termios); - ld = tty_ldisc_ref(tty); + if (opt & (TERMIOS_FLUSH|TERMIOS_WAIT)) { +retry_write_wait: + retval = wait_event_interruptible(tty->write_wait, !tty_chars_in_buffer(tty)); + if (retval < 0) + return retval; - if (ld != NULL) { - if ((opt & TERMIOS_FLUSH) && ld->ops->flush_buffer) - ld->ops->flush_buffer(tty); - tty_ldisc_deref(ld); - } + if (tty_write_lock(tty, 0) < 0) + goto retry_write_wait; - if (opt & TERMIOS_WAIT) { - tty_wait_until_sent(tty, 0); - if (signal_pending(current)) - return -ERESTARTSYS; - } + /* Racing writer? */ + if (tty_chars_in_buffer(tty)) { + tty_write_unlock(tty); + goto retry_write_wait; + } + + ld = tty_ldisc_ref(tty); + if (ld != NULL) { + if ((opt & TERMIOS_FLUSH) && ld->ops->flush_buffer) + ld->ops->flush_buffer(tty); + tty_ldisc_deref(ld); + } + + if ((opt & TERMIOS_WAIT) && tty->ops->wait_until_sent) { + tty->ops->wait_until_sent(tty, 0); + if (signal_pending(current)) { + tty_write_unlock(tty); + return -ERESTARTSYS; + } + } + + tty_set_termios(tty, &tmp_termios); - tty_set_termios(tty, &tmp_termios); + tty_write_unlock(tty); + } else { + tty_set_termios(tty, &tmp_termios); + } /* FIXME: Arguably if tmp_termios == tty->termios AND the actual requested termios was not tmp_termios then we may diff --git a/include/linux/tty.h b/include/linux/tty.h index eb33d948788cc..8be25caa97f67 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -480,6 +480,8 @@ extern void __stop_tty(struct tty_struct *tty); extern void stop_tty(struct tty_struct *tty); extern void __start_tty(struct tty_struct *tty); extern void start_tty(struct tty_struct *tty); +void tty_write_unlock(struct tty_struct *tty); +int tty_write_lock(struct tty_struct *tty, int ndelay); extern int tty_register_driver(struct tty_driver *driver); extern int tty_unregister_driver(struct tty_driver *driver); extern struct device *tty_register_device(struct tty_driver *driver, -- 2.39.5