tty: use new tty_insert_flip_string_and_push_buffer() in pty_write()

commit a501ab75e7624d133a5a3c7ec010687c8b961d23 upstream.

There is a race in pty_write(). pty_write() can be called in parallel
with e.g. ioctl(TIOCSTI) or ioctl(TCXONC) which also inserts chars to
the buffer. Provided, tty_flip_buffer_push() in pty_write() is called
outside the lock, it can commit inconsistent tail. This can lead to out
of bounds writes and other issues. See the Link below.

To fix this, we have to introduce a new helper called
tty_insert_flip_string_and_push_buffer(). It does both
tty_insert_flip_string() and tty_flip_buffer_commit() under the port
lock. It also calls queue_work(), but outside the lock. See
71a174b39f10 (pty: do tty_flip_buffer_push without port->lock in
pty_write) for the reasons.

Keep the helper internal-only (in drivers' tty.h). It is not intended to
be used widely.

Link: https://seclists.org/oss-sec/2022/q2/155
Fixes: 71a174b39f10 (pty: do tty_flip_buffer_push without port->lock in pty_write)
Cc: 一只狗 <chennbnbnb@gmail.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Suggested-by: Hillf Danton <hdanton@sina.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220707082558.9250-2-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 23368cec7ee844cffbf22f4f6d7418d2378c8ef7..16498f5fba64d41d18d32ecab5da208c210cd17c 100644 (file)
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -111,21 +111,11 @@ static void pty_unthrottle(struct tty_struct *tty)
 static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c)
 {
        struct tty_struct *to = tty->link;
-       unsigned long flags;
 
-       if (tty->stopped)
+       if (tty->stopped || !c)
                return 0;
 
-       if (c > 0) {
-               spin_lock_irqsave(&to->port->lock, flags);
-               /* Stuff the data into the input queue of the other end */
-               c = tty_insert_flip_string(to->port, buf, c);
-               spin_unlock_irqrestore(&to->port->lock, flags);
-               /* And shovel */
-               if (c)
-                       tty_flip_buffer_push(to->port);
-       }
-       return c;
+       return tty_insert_flip_string_and_push_buffer(to->port, buf, c);
 }
 
 /**

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index bf8d4160411c5e926d2cc12857539f2f993e6961..5bbc2e010b48358a7317bde531237be7620f2afd 100644 (file)
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -552,6 +552,37 @@ void tty_flip_buffer_push(struct tty_port *port)
 }
 EXPORT_SYMBOL(tty_flip_buffer_push);
 
+/**
+ * tty_insert_flip_string_and_push_buffer - add characters to the tty buffer and
+ *     push
+ * @port: tty port
+ * @chars: characters
+ * @size: size
+ *
+ * The function combines tty_insert_flip_string() and tty_flip_buffer_push()
+ * with the exception of properly holding the @port->lock.
+ *
+ * To be used only internally (by pty currently).
+ *
+ * Returns: the number added.
+ */
+int tty_insert_flip_string_and_push_buffer(struct tty_port *port,
+               const unsigned char *chars, size_t size)
+{
+       struct tty_bufhead *buf = &port->buf;
+       unsigned long flags;
+
+       spin_lock_irqsave(&port->lock, flags);
+       size = tty_insert_flip_string(port, chars, size);
+       if (size)
+               tty_flip_buffer_commit(buf->tail);
+       spin_unlock_irqrestore(&port->lock, flags);
+
+       queue_work(system_unbound_wq, &buf->work);
+
+       return size;
+}
+
 /**
  *     tty_buffer_init         -       prepare a tty buffer structure
  *     @port: tty port to initialise

diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index e183f74b19a09882c5ddd0e4365b46e368c6d1ac..c326bfdb5ec2c38d263b629cc818b1f281848071 100644 (file)
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -39,4 +39,7 @@ static inline int tty_insert_flip_string(struct tty_port *port,
 extern void tty_buffer_lock_exclusive(struct tty_port *port);
 extern void tty_buffer_unlock_exclusive(struct tty_port *port);
 
+int tty_insert_flip_string_and_push_buffer(struct tty_port *port,
+               const unsigned char *chars, size_t cnt);
+
 #endif /* _LINUX_TTY_FLIP_H */