Skip to content

Commit a501ab7

Browse files
Jiri Slabygregkh
authored andcommitted
tty: use new tty_insert_flip_string_and_push_buffer() in pty_write()
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 71a174b (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: 71a174b (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>
1 parent 716b105 commit a501ab7

File tree

3 files changed

+36
-12
lines changed

3 files changed

+36
-12
lines changed

drivers/tty/pty.c

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -111,21 +111,11 @@ static void pty_unthrottle(struct tty_struct *tty)
111111
static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c)
112112
{
113113
struct tty_struct *to = tty->link;
114-
unsigned long flags;
115114

116-
if (tty->flow.stopped)
115+
if (tty->flow.stopped || !c)
117116
return 0;
118117

119-
if (c > 0) {
120-
spin_lock_irqsave(&to->port->lock, flags);
121-
/* Stuff the data into the input queue of the other end */
122-
c = tty_insert_flip_string(to->port, buf, c);
123-
spin_unlock_irqrestore(&to->port->lock, flags);
124-
/* And shovel */
125-
if (c)
126-
tty_flip_buffer_push(to->port);
127-
}
128-
return c;
118+
return tty_insert_flip_string_and_push_buffer(to->port, buf, c);
129119
}
130120

131121
/**

drivers/tty/tty.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,7 @@ static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
111111

112112
ssize_t redirected_tty_write(struct kiocb *, struct iov_iter *);
113113

114+
int tty_insert_flip_string_and_push_buffer(struct tty_port *port,
115+
const unsigned char *chars, size_t cnt);
116+
114117
#endif

drivers/tty/tty_buffer.c

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,37 @@ void tty_flip_buffer_push(struct tty_port *port)
560560
}
561561
EXPORT_SYMBOL(tty_flip_buffer_push);
562562

563+
/**
564+
* tty_insert_flip_string_and_push_buffer - add characters to the tty buffer and
565+
* push
566+
* @port: tty port
567+
* @chars: characters
568+
* @size: size
569+
*
570+
* The function combines tty_insert_flip_string() and tty_flip_buffer_push()
571+
* with the exception of properly holding the @port->lock.
572+
*
573+
* To be used only internally (by pty currently).
574+
*
575+
* Returns: the number added.
576+
*/
577+
int tty_insert_flip_string_and_push_buffer(struct tty_port *port,
578+
const unsigned char *chars, size_t size)
579+
{
580+
struct tty_bufhead *buf = &port->buf;
581+
unsigned long flags;
582+
583+
spin_lock_irqsave(&port->lock, flags);
584+
size = tty_insert_flip_string(port, chars, size);
585+
if (size)
586+
tty_flip_buffer_commit(buf->tail);
587+
spin_unlock_irqrestore(&port->lock, flags);
588+
589+
queue_work(system_unbound_wq, &buf->work);
590+
591+
return size;
592+
}
593+
563594
/**
564595
* tty_buffer_init - prepare a tty buffer structure
565596
* @port: tty port to initialise

0 commit comments

Comments
 (0)