linux-toradex.git/drivers/tty/n_tty.c, branch v3.10.78

n_tty: Fix n_tty_write crash when echoing in raw mode

2014-05-13T11:59:40+00:00

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

TTY: Fix tty miss restart after we turn off flow-control

2013-05-20T19:15:59+00:00

I meet emacs hang in start if I do the operation below:
  1: echo 3 > /proc/sys/vm/drop_caches
  2: emacs BigFile
  3: Press CTRL-S follow 2 immediately

Then emacs hang on, CTRL-Q can't resume, the terminal
hang on, you can do nothing with this terminal except
close it.

The reason is before emacs takeover control the tty,
we use CTRL-S to XOFF it. Then when emacs takeover the
control, it may don't use the flow-control, so emacs hang.
This patch fix it.

This patch will fix a kind of strange tty relation hang problem,
I believe I meet it with vim in ssh, and also see below bug report:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=465823

Signed-off-by: Wang YanQing 
Cc: stable 
Signed-off-by: Greg Kroah-Hartman

n_tty: Fully initialize ldisc before restarting buffer work

2013-03-18T23:44:01+00:00

Buffer work may already be pending when the n_tty ldisc is re-opened,
eg., when setting the ldisc (via TIOCSETD ioctl) and when hanging up
the tty. Since n_tty_set_room() may restart buffer work, first ensure
the ldisc is completely initialized.

Factor n_tty_set_room() out of reset_buffer_flags() (only 2 callers)
and reorganize n_tty_open() to set termios last; buffer work will
be restarted there if necessary, after the char_map is properly
initialized.

Fixes this WARNING:

[  549.561769] ------------[ cut here ]------------
[  549.598755] WARNING: at drivers/tty/n_tty.c:160 n_tty_set_room+0xff/0x130()
[  549.604058] scheduling buffer work for halted ldisc
[  549.607741] Pid: 9417, comm: trinity-child28 Tainted: G      D W 3.7.0-next-20121217-sasha-00023-g8689ef9 #219
[  549.652580] Call Trace:
[  549.662754]  [] ? n_tty_set_room+0xff/0x130
[  549.665458]  [] warn_slowpath_common+0x87/0xb0
[  549.668257]  [] warn_slowpath_fmt+0x41/0x50
[  549.671007]  [] n_tty_set_room+0xff/0x130
[  549.673268]  [] reset_buffer_flags+0x137/0x150
[  549.675607]  [] n_tty_open+0x131/0x1c0
[  549.677699]  [] tty_ldisc_open.isra.5+0x54/0x70
[  549.680147]  [] tty_ldisc_hangup+0x11f/0x1e0
[  549.682409]  [] __tty_hangup+0x137/0x440
[  549.684634]  [] tty_vhangup+0x9/0x10
[  549.686443]  [] pty_close+0x14c/0x160
[  549.688446]  [] tty_release+0xd5/0x490
[  549.690460]  [] __fput+0x122/0x250
[  549.692577]  [] ____fput+0x9/0x10
[  549.694534]  [] task_work_run+0xb2/0xf0
[  549.696349]  [] do_exit+0x36d/0x580
[  549.698286]  [] ? syscall_trace_enter+0x24/0x2e0
[  549.702729]  [] do_group_exit+0x8a/0xc0
[  549.706775]  [] sys_exit_group+0x12/0x20
[  549.711088]  [] tracesys+0xe1/0xe6
[  549.728001] ---[ end trace 73eb41728f11f87e ]---

Reported-by: Sasha Levin 
Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

n_tty: Correct unthrottle-with-buffer-flush comments

2013-03-18T23:38:58+00:00

The driver is no longer unthrottled on buffer reset, so remove
comments that claim it is.

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

n_tty: Don't flush buffer when closing ldisc

2013-03-18T23:32:46+00:00

A buffer flush is both undesirable and unnecessary when the ldisc
is closing. A buffer flush performs the following:
 1. resets ldisc data fields to their initial state
 2. resets tty->receive_room to indicate more data can be sent
 3. schedules buffer work to receive more data
 4. signals a buffer flush has happened to linked pty in packet mode

Since the ldisc has been halted and the tty may soon be destructed,
buffer work must not be scheduled as that work might access
an invalid tty and ldisc state. Also, the ldisc read buffer is about
to be freed, so that's pointless.

Resetting the ldisc data fields is pointless as well since that
structure is about to be freed.

Resetting tty->receive_room is unnecessary, as it will be properly
reset if a new ldisc is reopened. Besides, resetting the original
receive_room value would be wrong since the read buffer will be
gone.

Since the packet mode flush is observable from userspace, this
behavior has been preserved.

The test jig originally authored by Ilya Zykov  and
signed off by him is included below. The test jig prompts the
following warnings which this patch fixes.

[   38.051111] ------------[ cut here ]------------
[   38.052113] WARNING: at drivers/tty/n_tty.c:160 n_tty_set_room.part.6+0x8b/0xa0()
[   38.053916] Hardware name: Bochs
[   38.054819] Modules linked in: netconsole configfs bnep rfcomm bluetooth parport_pc ppdev snd_hda_intel snd_hda_codec
snd_hwdep snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq psmouse snd_timer serio_raw mac_hid snd_seq_device
snd microcode lp parport virtio_balloon soundcore i2c_piix4 snd_page_alloc floppy 8139too 8139cp
[   38.059704] Pid: 1564, comm: pty_kill Tainted: G        W    3.7.0-next-20121130+ttydebug-xeon #20121130+ttydebug
[   38.061578] Call Trace:
[   38.062491]  [] warn_slowpath_common+0x7f/0xc0
[   38.063448]  [] warn_slowpath_null+0x1a/0x20
[   38.064439]  [] n_tty_set_room.part.6+0x8b/0xa0
[   38.065381]  [] n_tty_set_room+0x42/0x80
[   38.066323]  [] reset_buffer_flags+0x102/0x160
[   38.077508]  [] n_tty_flush_buffer+0x1d/0x90
[   38.078782]  [] ? default_spin_lock_flags+0x9/0x10
[   38.079734]  [] n_tty_close+0x24/0x60
[   38.080730]  [] tty_ldisc_close.isra.2+0x41/0x60
[   38.081680]  [] tty_ldisc_kill+0x3b/0x80
[   38.082618]  [] tty_ldisc_release+0x77/0xe0
[   38.083549]  [] tty_release+0x451/0x4d0
[   38.084525]  [] __fput+0xae/0x230
[   38.085472]  [] ____fput+0xe/0x10
[   38.086401]  [] task_work_run+0xc8/0xf0
[   38.087334]  [] do_exit+0x196/0x4b0
[   38.088304]  [] ? __dequeue_signal+0x6b/0xb0
[   38.089240]  [] do_group_exit+0x44/0xa0
[   38.090182]  [] get_signal_to_deliver+0x20d/0x4e0
[   38.091125]  [] do_signal+0x29/0x130
[   38.092096]  [] ? tty_ldisc_deref+0xe/0x10
[   38.093030]  [] ? tty_write+0xb7/0xf0
[   38.093976]  [] ? vfs_write+0xb3/0x180
[   38.094904]  [] do_notify_resume+0x80/0xc0
[   38.095830]  [] int_signal+0x12/0x17
[   38.096788] ---[ end trace 5f6f7a9651cd999b ]---

[ 2730.570602] ------------[ cut here ]------------
[ 2730.572130] WARNING: at drivers/tty/n_tty.c:160 n_tty_set_room+0x107/0x140()
[ 2730.574904] scheduling buffer work for halted ldisc
[ 2730.578303] Pid: 9691, comm: trinity-child15 Tainted: G        W 3.7.0-rc8-next-20121205-sasha-00023-g59f0d85 #207
[ 2730.588694] Call Trace:
[ 2730.590486]  [] ? n_tty_set_room+0x107/0x140
[ 2730.592559]  [] warn_slowpath_common+0x87/0xb0
[ 2730.595317]  [] warn_slowpath_fmt+0x41/0x50
[ 2730.599186]  [] n_tty_set_room+0x107/0x140
[ 2730.603141]  [] reset_buffer_flags+0x137/0x150
[ 2730.607166]  [] n_tty_flush_buffer+0x18/0x90
[ 2730.610123]  [] n_tty_close+0x1f/0x60
[ 2730.612068]  [] tty_ldisc_close.isra.4+0x52/0x60
[ 2730.614078]  [] tty_ldisc_reinit+0x3b/0x70
[ 2730.615891]  [] tty_ldisc_hangup+0x102/0x1e0
[ 2730.617780]  [] __tty_hangup+0x137/0x440
[ 2730.619547]  [] tty_vhangup+0x9/0x10
[ 2730.621266]  [] pty_close+0x14c/0x160
[ 2730.622952]  [] tty_release+0xd5/0x490
[ 2730.624674]  [] __fput+0x122/0x250
[ 2730.626195]  [] ____fput+0x9/0x10
[ 2730.627758]  [] task_work_run+0xb2/0xf0
[ 2730.629491]  [] do_exit+0x36d/0x580
[ 2730.631159]  [] do_group_exit+0x8a/0xc0
[ 2730.632819]  [] get_signal_to_deliver+0x501/0x5b0
[ 2730.634758]  [] do_signal+0x24/0x100
[ 2730.636412]  [] ? user_exit+0xa5/0xd0
[ 2730.638078]  [] ? trace_hardirqs_on_caller+0x118/0x140
[ 2730.640279]  [] ? trace_hardirqs_on+0xd/0x10
[ 2730.642164]  [] do_notify_resume+0x48/0xa0
[ 2730.643966]  [] int_signal+0x12/0x17
[ 2730.645672] ---[ end trace a40d53149c07fce0 ]---

/*
 * pty_thrash.c
 *
 * Based on original test jig by Ilya Zykov 
 *
 * Signed-off-by: Peter Hurley 
 * Signed-off-by: Ilya Zykov 
 */

static int fd;

static void error_exit(char *f, ...)
{
        va_list va;

        va_start(va, f);
        vprintf(f, va);
        printf(": %s\n", strerror(errno));
        va_end(va);

        if (fd >= 0)
                close(fd);

        exit(EXIT_FAILURE);
}

int main(int argc, char *argv[]) {
        int parent;
        char pts_name[24];
        int ptn, unlock;

        while (1) {

                fd = open("/dev/ptmx", O_RDWR);
                if (fd < 0)
                        error_exit("opening pty master");
                unlock = 0;
                if (ioctl(fd, TIOCSPTLCK, &unlock) < 0)
                        error_exit("unlocking pty pair");
                if (ioctl(fd, TIOCGPTN, &ptn) < 0)
                        error_exit("getting pty #");
                snprintf(pts_name, sizeof(pts_name), "/dev/pts/%d", ptn);

                child_id = fork();
                if (child_id == -1)
                        error_exit("forking child");

                if (parent) {
                        int err, id, status;
                        char buf[128];
                        int n;

                        n = read(fd, buf, sizeof(buf));
                        if (n < 0)
                                error_exit("master reading");
                        printf("%.*s\n", n-1, buf);

                        close(fd);

                        err = kill(child_id, SIGKILL);
                        if (err < 0)
                                error_exit("killing child");
                        id = waitpid(child_id, &status, 0);
                        if (id < 0 || id != child_id)
                                error_exit("waiting for child");

                } else { /* Child */

                        close(fd);
                        printf("Test cycle on slave pty %s\n", pts_name);
                        fd = open(pts_name, O_RDWR);
                        if (fd < 0)
                                error_exit("opening pty slave");

                        while (1) {
                                char pattern[] = "test\n";
                                if (write(fd, pattern, strlen(pattern)) < 0)
                                        error_exit("slave writing");
                        }

                }
        }

        /* never gets here */
        return 0;
}

Reported-by: Sasha Levin 
Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

n_tty: Factor packet mode status change for reuse

2013-03-18T23:32:46+00:00

Factor the packet mode status change from n_tty_flush_buffer
for use by follow-on patch.

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

tty: Add diagnostic for halted line discipline

2013-03-18T23:32:46+00:00

Flip buffer work must not be scheduled by the line discipline
after the line discipline has been halted; issue warning.

Note: drivers can still schedule flip buffer work.

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

n_tty: Lock access to tty->pgrp for POSIX job control

2013-03-18T23:13:59+00:00

Concurrent access to tty->pgrp must be protected with tty->ctrl_lock.
Also, as noted in the comments, reading current->signal->tty is
safe because either,
  1) current->signal->tty is assigned by current, or
  2) current->signal->tty is set to NULL.

NB: for reference, tty_check_change() implements a similar POSIX
check for the ioctls corresponding to tcflush(), tcdrain(),
tcsetattr(), tcsetpgrp(), tcflow() and tcsendbreak().

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

n_tty: Fix unsafe driver-side signals

2013-03-18T23:13:59+00:00

An ldisc reference is insufficient guarantee the foreground process
group is not in the process of being signalled from a hangup.

1) Reads of tty->pgrp must be locked with ctrl_lock
2) The group pid must be referenced for the duration of signalling.
   Because the driver-side is not process-context, a pid reference
   must be acquired.

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

n_tty: Fix stuck throttled driver

2013-03-18T23:11:59+00:00

As noted in the following comment:

  /* FIXME: there is a tiny race here if the receive room check runs
     before the other work executes and empties the buffer (upping
     the receiving room and unthrottling. We then throttle and get
     stuck. This has been observed and traced down by Vincent Pillet/
     We need to address this when we sort out out the rx path locking */

Use new safe throttle/unthrottle functions to re-evaluate conditions
if interrupted by the complement flow control function.

Reported-by: Vincent Pillet 
Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman