From patchwork Wed May 7 13:13:12 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Henriques X-Patchwork-Id: 346620 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 7F267140111; Wed, 7 May 2014 23:15:39 +1000 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1Wi1h9-0006Uz-Uh; Wed, 07 May 2014 13:15:35 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1Wi1h1-0006RD-Oo for kernel-team@lists.ubuntu.com; Wed, 07 May 2014 13:15:27 +0000 Received: from [188.250.142.22] (helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1Wi1h1-00008Z-DK; Wed, 07 May 2014 13:15:27 +0000 From: Luis Henriques To: linux-kernel@vger.kernel.org, stable@vger.kernel.org, kernel-team@lists.ubuntu.com Subject: [PATCH 3.11 69/70] n_tty: Fix n_tty_write crash when echoing in raw mode Date: Wed, 7 May 2014 14:13:12 +0100 Message-Id: <1399468393-10140-70-git-send-email-luis.henriques@canonical.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1399468393-10140-1-git-send-email-luis.henriques@canonical.com> References: <1399468393-10140-1-git-send-email-luis.henriques@canonical.com> X-Extended-Stable: 3.11 Cc: Peter Hurley , Jiri Slaby , Greg Kroah-Hartman , Linus Torvalds , Alan Cox X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com 3.11.10.10 -stable review patch. If anyone has any objections, please let me know. ------------------ From: Peter Hurley 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 Signed-off-by: Peter Hurley Signed-off-by: Jiri Slaby Cc: Linus Torvalds Cc: Alan Cox Signed-off-by: Greg Kroah-Hartman Signed-off-by: Luis Henriques --- drivers/tty/n_tty.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 4bf0fc0..10354a8 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -2066,8 +2066,12 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file, if (tty->ops->flush_chars) tty->ops->flush_chars(tty); } else { + struct n_tty_data *ldata = tty->disc_data; + while (nr > 0) { + mutex_lock(&ldata->output_lock); c = tty->ops->write(tty, b, nr); + mutex_unlock(&ldata->output_lock); if (c < 0) { retval = c; goto break_out;