This is a note to let you know that I've just added the patch titled
tty: Prevent writing chars during tcsetattr TCSADRAIN/FLUSH
to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
tty-prevent-writing-chars-during-tcsetattr-tcsadrain-flush.patch
and it can be found in the queue-4.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From stable-owner@xxxxxxxxxxxxxxx Thu May 11 14:33:03 2023
From: "Ilpo Järvinen" <ilpo.jarvinen@xxxxxxxxxxxxxxx>
Date: Thu, 11 May 2023 15:32:43 +0300
Subject: tty: Prevent writing chars during tcsetattr TCSADRAIN/FLUSH
To: stable@xxxxxxxxxxxxxxx
Cc: "Ilpo Järvinen" <ilpo.jarvinen@xxxxxxxxxxxxxxx>, "Greg Kroah-Hartman" <gregkh@xxxxxxxxxxxxxxxxxxx>
Message-ID: <20230511123244.38514-1-ilpo.jarvinen@xxxxxxxxxxxxxxx>
From: "Ilpo Järvinen" <ilpo.jarvinen@xxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
Link: https://lore.kernel.org/r/20230317113318.31327-2-ilpo.jarvinen@xxxxxxxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
(cherry picked from commit 094fb49a2d0d6827c86d2e0840873e6db0c491d2)
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/tty/tty_io.c | 4 ++--
drivers/tty/tty_ioctl.c | 45 +++++++++++++++++++++++++++++++++------------
include/linux/tty.h | 2 ++
3 files changed, 37 insertions(+), 14 deletions(-)
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -874,13 +874,13 @@ static ssize_t tty_read(struct file *fil
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, POLLOUT);
}
-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)
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -396,21 +396,42 @@ static int set_termios(struct tty_struct
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);
+ }
- tty_set_termios(tty, &tmp_termios);
+ 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_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
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -479,6 +479,8 @@ extern void __stop_tty(struct tty_struct
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,
Patches currently in stable-queue which might be from stable-owner@xxxxxxxxxxxxxxx are
queue-4.14/serial-8250-fix-serial8250_tx_empty-race-with-dma-tx.patch
queue-4.14/tty-prevent-writing-chars-during-tcsetattr-tcsadrain-flush.patch
