All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Peter Hurley <peter@hurleysoftware.com>,
	Jiri Slaby <jslaby@suse.cz>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Ben Hutchings <ben@decadent.org.uk>
Subject: [PATCH 3.4 2/9] n_tty: Fix n_tty_write crash when echoing in raw mode
Date: Fri, 16 May 2014 15:55:25 -0700	[thread overview]
Message-ID: <20140516225458.704630668@linuxfoundation.org> (raw)
In-Reply-To: <20140516225458.512693003@linuxfoundation.org>

3.4-stable review patch.  If anyone has any objections, please let me know.

------------------

commit 4291086b1f081b869c6d79e5b7441633dc3ace00 upstream.

The tty atomic_write_lock does not provide an exclusion guarantee for
the tty driver if the termios settings are LECHO & !OPOST.  And since
it is unexpected and not allowed to call TTY buffer helpers like
tty_insert_flip_string concurrently, this may lead to crashes when
concurrect writers call pty_write. In that case the following two
writers:
* the ECHOing from a workqueue and
* pty_write from the process
race and can overflow the corresponding TTY buffer like follows.

If we look into tty_insert_flip_string_fixed_flag, there is:
  int space = __tty_buffer_request_room(port, goal, flags);
  struct tty_buffer *tb = port->buf.tail;
  ...
  memcpy(char_buf_ptr(tb, tb->used), chars, space);
  ...
  tb->used += space;

so the race of the two can result in something like this:
              A                                B
__tty_buffer_request_room
                                  __tty_buffer_request_room
memcpy(buf(tb->used), ...)
tb->used += space;
                                  memcpy(buf(tb->used), ...) ->BOOM

B's memcpy is past the tty_buffer due to the previous A's tb->used
increment.

Since the N_TTY line discipline input processing can output
concurrently with a tty write, obtain the N_TTY ldisc output_lock to
serialize echo output with normal tty writes.  This ensures the tty
buffer helper tty_insert_flip_string is not called concurrently and
everything is fine.

Note that this is nicely reproducible by an ordinary user using
forkpty and some setup around that (raw termios + ECHO). And it is
present in kernels at least after commit
d945cb9cce20ac7143c2de8d88b187f62db99bdc (pty: Rework the pty layer to
use the normal buffering logic) in 2.6.31-rc3.

js: add more info to the commit log
js: switch to bool
js: lock unconditionally
js: lock only the tty->ops->write call

References: CVE-2014-0196
Reported-and-tested-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[bwh: Backported to 3.2: output_lock is a member of struct tty_struct]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/tty/n_tty.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1996,7 +1996,9 @@ static ssize_t n_tty_write(struct tty_st
 				tty->ops->flush_chars(tty);
 		} else {
 			while (nr > 0) {
+				mutex_lock(&tty->output_lock);
 				c = tty->ops->write(tty, b, nr);
+				mutex_unlock(&tty->output_lock);
 				if (c < 0) {
 					retval = c;
 					goto break_out;



  parent reply	other threads:[~2014-05-16 22:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-16 22:55 [PATCH 3.4 0/9] 3.4.91-stable review Greg Kroah-Hartman
2014-05-16 22:55 ` [PATCH 3.4 1/9] SCSI: megaraid: missing bounds check in mimd_to_kioc() Greg Kroah-Hartman
2014-05-16 22:55 ` Greg Kroah-Hartman [this message]
2014-05-16 22:55 ` [PATCH 3.4 3/9] blktrace: fix accounting of partially completed requests Greg Kroah-Hartman
2014-05-16 22:55 ` [PATCH 3.4 4/9] netfilter: nf_conntrack: reserve two bytes for nf_ct_ext->len Greg Kroah-Hartman
2014-05-16 22:55 ` [PATCH 3.4 5/9] net: Add net_ratelimited_function and net_<level>_ratelimited macros Greg Kroah-Hartman
2014-05-16 22:55 ` [PATCH 3.4 6/9] netfilter: Cant fail and free after table replacement Greg Kroah-Hartman
2014-05-16 22:55 ` [PATCH 3.4 7/9] tracepoint: Do not waste memory on mods with no tracepoints Greg Kroah-Hartman
2014-05-16 22:55 ` [PATCH 3.4 8/9] powerpc: Add vr save/restore functions Greg Kroah-Hartman
2014-05-16 22:55 ` [PATCH 3.4 9/9] tgafb: fix mode setting with fbset Greg Kroah-Hartman
2014-05-17 15:44 ` [PATCH 3.4 0/9] 3.4.91-stable review Guenter Roeck
2014-05-17 17:25   ` Greg Kroah-Hartman
2014-05-18 10:09   ` Satoru Takeuchi
2014-05-18 12:23     ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140516225458.704630668@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=ben@decadent.org.uk \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter@hurleysoftware.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.