mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ben Hutchings <ben@decadent.org.uk>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: akpm@linux-foundation.org, "Alan Cox" <alan@lxorguk.ukuu.org.uk>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"" <stable@vger.kernel.org>, "Jiri Slaby" <jslaby@suse.cz>,
	"Peter Hurley" <peter@hurleysoftware.com>
Subject: [PATCH 3.2 32/34] n_tty: Fix n_tty_write crash when echoing in raw mode
Date: Fri, 16 May 2014 13:47:21 +0100	[thread overview]
Message-ID: <lsq.1400244441.938704882@decadent.org.uk> (raw)
In-Reply-To: <lsq.1400244440.390103756@decadent.org.uk>

3.2.59-rc1 review patch.  If anyone has any objections, please let me know.

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

From: Peter Hurley <peter@hurleysoftware.com>

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>
Cc: <stable@vger.kernel.org>
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 | 4 ++++
 1 file changed, 4 insertions(+)

--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1997,7 +1997,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 12:49 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-16 12:47 [PATCH 3.2 00/34] 3.2.59-rc1 review Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 05/34] locks: allow __break_lease to sleep even when break_time is 0 Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 04/34] parisc: fix epoll_pwait syscall on compat kernel Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 01/34] ext4: FIBMAP ioctl causes BUG_ON due to handle EXT_MAX_BLOCKS Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 03/34] ext4: use i_size_read in ext4_unaligned_aio() Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 02/34] ext4: note the error in ext4_end_bio() Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 08/34] USB: serial: ftdi_sio: add id for Brainboxes serial cards Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 07/34] staging: r8712u: Fix case where ethtype was never obtained and always be checked against 0 Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 14/34] nfsd: set timeparms.to_maxval in setup_callback_client Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 28/34] usb: option: add Olivetti Olicard 500 Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 26/34] usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 22/34] Btrfs: Don't allocate inode that is already in use Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 12/34] USB: pl2303: add ids for Hewlett-Packard HP POS pole displays Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 34/34] [2/2] floppy: don't write kernel-only members to FDRAWCMD ioctl output Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 10/34] usb: option driver, add support for Telit UE910v2 Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 31/34] rtl8192ce: Fix null dereference in watchdog Ben Hutchings
2014-05-16 14:20   ` Larry Finger
2014-05-16 15:38     ` Ben Hutchings
2014-05-16 16:08       ` Larry Finger
2014-05-16 12:47 ` Ben Hutchings [this message]
2014-05-16 12:47 ` [PATCH 3.2 21/34] USB: serial: fix sysfs-attribute removal deadlock Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 19/34] mm: make fixup_user_fault() check the vma access rights too Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 25/34] usb: xhci: Prefer endpoint context dequeue pointer over stopped_trb Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 29/34] usb: option: add Alcatel L800MA Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 24/34] xhci: For streams the css flag most be read from the stream-ctx on ep stop Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 11/34] USB: cp210x: Add 8281 (Nanotec Plug & Drive) Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 13/34] USB: cdc-acm: Remove Motorola/Telit H24 serial interfaces from ACM driver Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 16/34] mm/hugetlb.c: add cond_resched_lock() in return_unused_surplus_pages() Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 33/34] [1/2] floppy: ignore kernel-only members in FDRAWCMD ioctl input Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 23/34] Btrfs: fix inode caching vs tree log Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 18/34] Input: synaptics - add min/max quirk for ThinkPad T431s, L440, L540, S1 Yoga and X1 Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 09/34] Revert "USB: serial: add usbid for dell wwan card to sierra.c" Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 06/34] mlx4_en: don't use napi_synchronize inside mlx4_en_netpoll Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 15/34] libata/ahci: accommodate tag ordered controllers Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 20/34] ARM: 8027/1: fix do_div() bug in big-endian systems Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 27/34] USB: io_ti: fix firmware download on big-endian machines Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 30/34] usb: option: add and update a number of CMOTech devices Ben Hutchings
2014-05-16 12:47 ` [PATCH 3.2 17/34] dmi: add support for exact DMI matches in addition to substring matching Ben Hutchings
2014-05-16 12:59 ` [PATCH 3.2 00/34] 3.2.59-rc1 review Ben Hutchings
2014-05-16 15:55 ` Guenter Roeck
2014-05-18 13:55   ` Ben Hutchings

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=lsq.1400244441.938704882@decadent.org.uk \
    --to=ben@decadent.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®