Commit Graph

39 Commits

Author SHA1 Message Date
Peter Hurley b66f4fa509 n_tty: Fully initialize ldisc before restarting buffer work
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]  [<ffffffff81c432cf>] ? n_tty_set_room+0xff/0x130
[  549.665458]  [<ffffffff8110cae7>] warn_slowpath_common+0x87/0xb0
[  549.668257]  [<ffffffff8110cb71>] warn_slowpath_fmt+0x41/0x50
[  549.671007]  [<ffffffff81c432cf>] n_tty_set_room+0xff/0x130
[  549.673268]  [<ffffffff81c44597>] reset_buffer_flags+0x137/0x150
[  549.675607]  [<ffffffff81c45b71>] n_tty_open+0x131/0x1c0
[  549.677699]  [<ffffffff81c47824>] tty_ldisc_open.isra.5+0x54/0x70
[  549.680147]  [<ffffffff81c482bf>] tty_ldisc_hangup+0x11f/0x1e0
[  549.682409]  [<ffffffff81c3fa17>] __tty_hangup+0x137/0x440
[  549.684634]  [<ffffffff81c3fd49>] tty_vhangup+0x9/0x10
[  549.686443]  [<ffffffff81c4a42c>] pty_close+0x14c/0x160
[  549.688446]  [<ffffffff81c41225>] tty_release+0xd5/0x490
[  549.690460]  [<ffffffff8127d8a2>] __fput+0x122/0x250
[  549.692577]  [<ffffffff8127d9d9>] ____fput+0x9/0x10
[  549.694534]  [<ffffffff811348c2>] task_work_run+0xb2/0xf0
[  549.696349]  [<ffffffff81113c6d>] do_exit+0x36d/0x580
[  549.698286]  [<ffffffff8107d964>] ? syscall_trace_enter+0x24/0x2e0
[  549.702729]  [<ffffffff81113f4a>] do_group_exit+0x8a/0xc0
[  549.706775]  [<ffffffff81113f92>] sys_exit_group+0x12/0x20
[  549.711088]  [<ffffffff83cfab18>] tracesys+0xe1/0xe6
[  549.728001] ---[ end trace 73eb41728f11f87e ]---

Reported-by: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:44:01 -07:00
Peter Hurley 25518c68b3 n_tty: Correct unthrottle-with-buffer-flush comments
The driver is no longer unthrottled on buffer reset, so remove
comments that claim it is.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:38:58 -07:00
Peter Hurley 79901317ce n_tty: Don't flush buffer when closing ldisc
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 <ilya@ilyx.ru> 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]  [<ffffffff81058b4f>] warn_slowpath_common+0x7f/0xc0
[   38.063448]  [<ffffffff81058baa>] warn_slowpath_null+0x1a/0x20
[   38.064439]  [<ffffffff8142dc2b>] n_tty_set_room.part.6+0x8b/0xa0
[   38.065381]  [<ffffffff8142dc82>] n_tty_set_room+0x42/0x80
[   38.066323]  [<ffffffff8142e6f2>] reset_buffer_flags+0x102/0x160
[   38.077508]  [<ffffffff8142e76d>] n_tty_flush_buffer+0x1d/0x90
[   38.078782]  [<ffffffff81046569>] ? default_spin_lock_flags+0x9/0x10
[   38.079734]  [<ffffffff8142e804>] n_tty_close+0x24/0x60
[   38.080730]  [<ffffffff81431b61>] tty_ldisc_close.isra.2+0x41/0x60
[   38.081680]  [<ffffffff81431bbb>] tty_ldisc_kill+0x3b/0x80
[   38.082618]  [<ffffffff81432a07>] tty_ldisc_release+0x77/0xe0
[   38.083549]  [<ffffffff8142b781>] tty_release+0x451/0x4d0
[   38.084525]  [<ffffffff811950be>] __fput+0xae/0x230
[   38.085472]  [<ffffffff8119524e>] ____fput+0xe/0x10
[   38.086401]  [<ffffffff8107aa88>] task_work_run+0xc8/0xf0
[   38.087334]  [<ffffffff8105ea56>] do_exit+0x196/0x4b0
[   38.088304]  [<ffffffff8106c77b>] ? __dequeue_signal+0x6b/0xb0
[   38.089240]  [<ffffffff8105ef34>] do_group_exit+0x44/0xa0
[   38.090182]  [<ffffffff8106f43d>] get_signal_to_deliver+0x20d/0x4e0
[   38.091125]  [<ffffffff81016979>] do_signal+0x29/0x130
[   38.092096]  [<ffffffff81431a9e>] ? tty_ldisc_deref+0xe/0x10
[   38.093030]  [<ffffffff8142a317>] ? tty_write+0xb7/0xf0
[   38.093976]  [<ffffffff81193f53>] ? vfs_write+0xb3/0x180
[   38.094904]  [<ffffffff81016b20>] do_notify_resume+0x80/0xc0
[   38.095830]  [<ffffffff81700492>] 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]  [<ffffffff81c41d77>] ? n_tty_set_room+0x107/0x140
[ 2730.592559]  [<ffffffff8110c827>] warn_slowpath_common+0x87/0xb0
[ 2730.595317]  [<ffffffff8110c8b1>] warn_slowpath_fmt+0x41/0x50
[ 2730.599186]  [<ffffffff81c41d77>] n_tty_set_room+0x107/0x140
[ 2730.603141]  [<ffffffff81c42fe7>] reset_buffer_flags+0x137/0x150
[ 2730.607166]  [<ffffffff81c43018>] n_tty_flush_buffer+0x18/0x90
[ 2730.610123]  [<ffffffff81c430af>] n_tty_close+0x1f/0x60
[ 2730.612068]  [<ffffffff81c461f2>] tty_ldisc_close.isra.4+0x52/0x60
[ 2730.614078]  [<ffffffff81c462ab>] tty_ldisc_reinit+0x3b/0x70
[ 2730.615891]  [<ffffffff81c46db2>] tty_ldisc_hangup+0x102/0x1e0
[ 2730.617780]  [<ffffffff81c3e537>] __tty_hangup+0x137/0x440
[ 2730.619547]  [<ffffffff81c3e869>] tty_vhangup+0x9/0x10
[ 2730.621266]  [<ffffffff81c48f1c>] pty_close+0x14c/0x160
[ 2730.622952]  [<ffffffff81c3fd45>] tty_release+0xd5/0x490
[ 2730.624674]  [<ffffffff8127fbe2>] __fput+0x122/0x250
[ 2730.626195]  [<ffffffff8127fd19>] ____fput+0x9/0x10
[ 2730.627758]  [<ffffffff81134602>] task_work_run+0xb2/0xf0
[ 2730.629491]  [<ffffffff811139ad>] do_exit+0x36d/0x580
[ 2730.631159]  [<ffffffff81113c8a>] do_group_exit+0x8a/0xc0
[ 2730.632819]  [<ffffffff81127351>] get_signal_to_deliver+0x501/0x5b0
[ 2730.634758]  [<ffffffff8106de34>] do_signal+0x24/0x100
[ 2730.636412]  [<ffffffff81204865>] ? user_exit+0xa5/0xd0
[ 2730.638078]  [<ffffffff81183cd8>] ? trace_hardirqs_on_caller+0x118/0x140
[ 2730.640279]  [<ffffffff81183d0d>] ? trace_hardirqs_on+0xd/0x10
[ 2730.642164]  [<ffffffff8106df78>] do_notify_resume+0x48/0xa0
[ 2730.643966]  [<ffffffff83cdff6a>] int_signal+0x12/0x17
[ 2730.645672] ---[ end trace a40d53149c07fce0 ]---

/*
 * pty_thrash.c
 *
 * Based on original test jig by Ilya Zykov <ilya@ilyx.ru>
 *
 * Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
 * Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
 */

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 <levinsasha928@gmail.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:32:46 -07:00
Peter Hurley a30737ab7d n_tty: Factor packet mode status change for reuse
Factor the packet mode status change from n_tty_flush_buffer
for use by follow-on patch.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:32:46 -07:00
Peter Hurley 21622939fc tty: Add diagnostic for halted line discipline
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 <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:32:46 -07:00
Peter Hurley 01a5e440c9 n_tty: Lock access to tty->pgrp for POSIX job control
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 <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:13:59 -07:00
Peter Hurley 8c985d18b1 n_tty: Fix unsafe driver-side signals
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 <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:13:59 -07:00
Peter Hurley e91e52e428 n_tty: Fix stuck throttled driver
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 <vincentx.pillet@intel.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:11:59 -07:00
Peter Hurley c828f679ee n_tty: Inline check_unthrottle() at lone call site
2-line function check_unthrottle() is now only called from
n_tty_read(); merge into caller.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:11:59 -07:00
George Spelvin 593fb1ae45 pps: Move timestamp read into PPS code proper
The PPS (Pulse-Per-Second) line discipline has developed a number of
unhealthy attachments to core tty data and functions, ultimately leading
to its breakage.

The previous patches fixed the crashing.  This one reduces coupling further
by eliminating the timestamp parameter from the dcd_change ldisc method.
This reduces header file linkage and makes the extension more generic,
and the timestamp read is delayed only slightly, from just before the
ldisc->ops->dcd_change method call to just after.

Fix attendant build breakage in
    drivers/tty/n_tty.c
    drivers/tty/tty_buffer.c
    drivers/staging/speakup/selection.c
    drivers/staging/dgrp/dgrp_*.c

Cc: William Hubbs <w.d.hubbs@gmail.com>
Cc: Chris Brannon <chris@the-brannons.com>
Cc: Kirk Reiser <kirk@braille.uwo.ca>
Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: George Spelvin <linux@horizon.com>
Acked-by: Rodolfo Giometti <giometti@enneenne.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-02-13 10:13:58 -08:00
Ivo Sieben 98001214c0 tty: Use raw spin lock to protect the TTY read section
The "normal" spin lock that guards the N_TTY line discipline read section
is replaced by a raw spin lock.

On a PREEMP_RT system this prevents unwanted scheduling overhead when data is
read at the same time as data is being received: while RX IRQ threaded handling
is busy a TTY read call is performed from a RT priority > threaded IRQ priority.
The read call tries to take the read section spin lock (held by the threaded
IRQ) which blocks and causes a context switch to/from the threaded IRQ handler
until the spin lock is unlocked.

Signed-off-by: Ivo Sieben <meltedpianoman@gmail.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-02-04 15:05:04 -08:00
Greg Kroah-Hartman 1651d0a9be Revert "n_tty: Unthrottle tty when flushing read buffer"
This reverts commit 58f82be334.

This was fixed by a previous patch already.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-01-29 23:35:35 -05:00
Karthik Manamcheri 58f82be334 n_tty: Unthrottle tty when flushing read buffer
When the tty input buffer is full and thereby throttled,
flushing/resetting the read buffer should unthrottle to allow more
data to be received.

Signed-off-by: Karthik Manamcheri <Karthik.Manamcheri@ni.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-01-25 08:52:23 -08:00
Sasha Levin cadf748690 tty: add missing newlines to WARN_RATELIMIT
WARN_RATELIMIT() expects the warning to end with a newline if one
is needed.

Not doing so results in odd looking warnings such as:

[ 1339.454272] tty is NULLPid: 7147, comm: kworker/4:0 Tainted: G        W    3.7.0-rc2-next-20121025-sasha-00001-g673f98e-dirty #75

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-25 11:30:27 -07:00
Jiri Slaby ecbbfd44a0 TTY: move tty buffers to tty_port
So this is it. The big step why we did all the work over the past
kernel releases. Now everything is prepared, so nothing protects us
from doing that big step.

           |  |            \  \ nnnn/^l      |  |
           |  |             \  /     /       |  |
           |  '-,.__   =>    \/   ,-`    =>  |  '-,.__
           | O __.´´)        (  .`           | O __.´´)
            ~~~   ~~          ``              ~~~   ~~
The buffers are now in the tty_port structure and we can start
teaching the buffer helpers (insert char/string, flip etc.) to use
tty_port instead of tty_struct all around.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:58:28 -07:00
Jiri Slaby 57c941212d TTY: n_tty, propagate n_tty_data
In some funtions we need only n_tty_data, so pass it down directly in
case tty is not needed there.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:53:01 -07:00
Jiri Slaby bddc7152f6 TTY: move ldisc data from tty_struct: locks
atomic_write_lock is not n_tty specific, so move it up in the
tty_struct.

And since these are the last ones to move, remove also the comment
saying there are some ldisc' members. There are none now.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:53:01 -07:00
Jiri Slaby ba2e68ac61 TTY: move ldisc data from tty_struct: read_* and echo_* and canon_* stuff
All the ring-buffers...

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:53:01 -07:00
Jiri Slaby 3fe780b379 TTY: move ldisc data from tty_struct: bitmaps
Here we move bitmaps and use DECLARE_BITMAP to declare them in the new
structure. And instead of memset, we use bitmap_zero as it is more
appropriate.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:53:00 -07:00
Jiri Slaby 53c5ee2cfb TTY: move ldisc data from tty_struct: simple members
Here we start moving all the n_tty related bits from tty_struct to
the newly defined n_tty_data struct in n_tty proper.

In this patch primitive members and bits are moved. The rest will be
done per-partes in the next patches.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:53:00 -07:00
Jiri Slaby 70ece7a731 TTY: n_tty, add ldisc data to n_tty
All n_tty related members from tty_struct will be moved here.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:50:54 -07:00
Jiri Slaby 6c633f27cc TTY: audit, stop accessing tty->icount
This is a private member of n_tty. Stop accessing it. Instead, take is
as an argument.

This is needed to allow clean switch of the private members to a
separate private structure of n_tty.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:50:54 -07:00
Jiri Slaby 3383427a7b TTY: n_tty, remove bogus checks
* BUG_ON(!tty) in n_tty_set_termios -- it cannot be called with tty ==
  NULL. It is called from two call sites. First, from n_tty_open where
  we have a valid tty. Second, as ld->ops->set_termios from
  tty_set_termios. But there we have a valid tty too.
* if (!tty) in n_tty_open -- why would the TTY layer call ldisc's
  open with an invalid TTY? No it indeed does not. All call sites have
  a tty and dereference that.
* BUG_ON(!tty->read_buf) in n_tty_read -- this used to be a valid
  check. The ldisc handling was broken some time ago when I added the
  check to ensure everything is OK. It still can catch the case, but
  no later than we move the buffer to ldisc data. Then there will be
  no read_buf in tty_struct, i.e. nothing to check for.
* if (!tty->read_buf) in n_tty_receive_buf -- this should never
  happen. All callers of ldisc->ops->receive_ops should hold a
  reference to an ldisc and close (which frees read_buf) cannot be
  called until the reference is dropped.
* if (WARN_ON(!tty->read_buf)) in n_tty_read -- the same as in the
  previous case.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:50:53 -07:00
Jiri Slaby b91939f528 TTY: n_tty, simplify read_buf+echo_buf allocation
ldisc->open and close are called only once and cannot cross. So the
tests in open and close are superfluous. Remove them. (But leave sets
to NULL to ensure there is not a bug somewhere.)

And when the tests are gone, handle properly failures in open. We
leaked read_buf if allocation of echo_buf failed before. Now this is
not the case anymore.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-22 16:50:53 -07:00
Stanislav Kozina e9490e93c1 Remove BUG_ON from n_tty_read()
Change the BUG_ON to WARN_ON and return in case of tty->read_buf==NULL. We want to track a
couple of long standing reports of this but at the same time we can avoid killing the box.

Signed-off-by: Stanislav Kozina <skozina@redhat.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Cc: Horses <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-08-16 11:53:14 -07:00
Jaeden Amero 090abf7b91 n_tty: Don't lose characters when PARMRK is enabled
When PARMRK is set and large transfers of characters that will get
marked are being received, n_tty could drop data silently (i.e.
without reporting any error to the client). This is because
characters have the potential to take up to three bytes in the line
discipline (when they get marked with parity or framing errors), but
the amount of free space reported to tty_buffer flush_to_ldisc (via
tty->receive_room) is based on the pre-marked data size.

With this patch, the n_tty layer will no longer assume that each byte
will only take up one byte in the line discipline. Instead, it will
make an overly conservative estimate that each byte will take up
three bytes in the line discipline when PARMRK is set.

Signed-off-by: Jaeden Amero <jaeden.amero@ni.com>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-08-10 13:14:54 -07:00
Stanislav Kozina 00aaae033e tty: Fix possible race in n_tty_read()
Fix possible panic caused by unlocked access to tty->read_cnt in
while-loop condition in n_tty_read().

Signed-off-by: Stanislav Kozina <skozina@redhat.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-08-10 13:13:11 -07:00
Alan Cox adc8d746ca tty: move the termios object into the tty
This will let us sort out a whole pile of tty related races. The
alternative would be to keep points and refcount the termios objects.
However
1. They are tiny anyway
2. Many devices don't use the stored copies
3. We can remove a pty special case

Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-07-16 13:00:41 -07:00
Alan Cox 0a44ab41eb tty: note race we need to fix
This was identified by Vincent Pillet with a high speed interface that uses
low latency mode. In the low latency case we have a tiny race but it can
be hit.

Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-06-26 19:25:38 -07:00
Jiri Slaby 3fa10cc83f TTY: n_tty, do not dereference user buffer
copy_from_read_buf currently copies data to a user buffer and then
checks if the data is single EOF. But it checks it by accessing the
user buffer. First, the buffer may be changed by other threads of the
user program already. Second, it accesses the buffer without any
checks. It might be write-only for example.

Fix this by inspecting contents of the tty (kernel) buffer instead.
Note that "n == 1" is necessary, but not sufficient. But we check
later that there is nothing left by "!tty->read_cnt" condition.

There is still an issue with the current code that EOF being wrapped
to the start of the circular buffer will result in an inappropriate
losing of the EOF character. But this is not intended to be fixed by
this patch.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-by: Emil Goode <emilgoode@gmail.com>
Cc: Howard Chu <hyc@symas.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-29 22:13:54 -04:00
David Howells 9ffc93f203 Remove all #inclusions of asm/system.h
Remove all #inclusions of asm/system.h preparatory to splitting and killing
it.  Performed with the following command:

perl -p -i -e 's!^#\s*include\s*<asm/system[.]h>.*\n!!' `grep -Irl '^#\s*include\s*<asm/system[.]h>' *`

Signed-off-by: David Howells <dhowells@redhat.com>
2012-03-28 18:30:03 +01:00
Thorsten Wißmann bbd20759d1 drivers/tty: Remove unneeded spaces
coding style fixes in n_tty.c

Signed-off-by: Maximilian Krger <maxfragg@gmail.com>
Signed-off-by: Thorsten Wimann <re06huxa@cip.cs.fau.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2011-12-09 19:11:35 -08:00
Linus Torvalds d5ef642355 Merge branch 'tty-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty-2.6
* 'tty-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty-2.6: (26 commits)
  amba pl011: workaround for uart registers lockup
  n_gsm: fix the wrong FCS handling
  pch_uart: add missing comment about OKI ML7223
  pch_uart: Add MSI support
  tty: fix "IRQ45: nobody cared"
  PTI feature to allow user to name and mark masterchannel request.
  0 for o PTI Makefile bug.
  tty: serial: samsung.c remove legacy PM code.
  SERIAL: SC26xx: Fix link error.
  serial: mrst_max3110: initialize waitqueue earlier
  mrst_max3110: Change max missing message priority.
  tty: s5pv210: Add delay loop on fifo reset function for UART
  tty/serial: Fix XSCALE serial ports, e.g. ce4100
  serial: bfin_5xx: fix off-by-one with resource size
  drivers/tty: use printk_ratelimited() instead of printk_ratelimit()
  tty: n_gsm: Added refcount usage to gsm_mux and gsm_dlci structs
  tty: n_gsm: Add raw-ip support
  tty: n_gsm: expose gsmtty device nodes at ldisc open time
  pch_phub: Fix register miss-setting issue
  serial: 8250, increase PASS_LIMIT
  ...
2011-07-25 23:09:27 -07:00
Andrew McGregor 7b292b4bf9 tty: fix "IRQ45: nobody cared"
Unthrottling the TTY during close ends up enabling interrupts
on a device not on the active list, which will never have the
interrupts cleared.  Doctor, it hurts when I do this.

>>> On 6/2/2011 at 01:56 AM, in message <20110601145608.3e586e16@bob.linux.org.uk>, Alan Cox <alan@linux.intel.com> wrote:
> On Wed, 01 Jun 2011 10:34:07 +1200
> "andrew mcgregor" <andrew.mcgregor@alliedtelesis.co.nz> wrote:
> > The LKML message
> > http://kerneltrap.org/mailarchive/linux-kernel/2010/2/25/4541847 from
> > February doesn't seem to have been resolved since.  We struck the
> > issue, and the patch below (against 2.6.32) fixes it.  Should I
> > supply a patch against 3.0.0rc?
>
> I think that would be sensible. I don't actually see how you hit it as
> the IRQ ought to be masked by then but it's certainly wrong for n_tty
> to be calling into check_unthrottle at that point.
>
> So yes please send a patch with a suitable Signed-off-by: line to
> linux-serial and cc GregKH <greg@kroah.com> as well.
>
> Alan

Signed-off-by: Andrew McGregor <andrew.mcgregor@alliedtelesis.co.nz>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2011-07-01 15:40:19 -07:00
Jiri Slaby 2872628680 TTY: ntty, add one more sanity check
With the previous patch, we fixed another bug where read_buf was freed
while we still was in n_tty_read. We currently check whether read_buf
is NULL at the start of the function. Add one more check after we wake
up from waiting for input.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2011-06-07 10:36:47 -07:00
Linus Torvalds 55db4c64ed Revert "tty: make receive_buf() return the amout of bytes received"
This reverts commit b1c43f82c5.

It was broken in so many ways, and results in random odd pty issues.

It re-introduced the buggy schedule_work() in flush_to_ldisc() that can
cause endless work-loops (see commit a5660b41af6a: "tty: fix endless
work loop when the buffer fills up").

It also used an "unsigned int" return value fo the ->receive_buf()
function, but then made multiple functions return a negative error code,
and didn't actually check for the error in the caller.

And it didn't actually work at all.  BenH bisected down odd tty behavior
to it:
  "It looks like the patch is causing some major malfunctions of the X
   server for me, possibly related to PTYs.  For example, cat'ing a
   large file in a gnome terminal hangs the kernel for -minutes- in a
   loop of what looks like flush_to_ldisc/workqueue code, (some ftrace
   data in the quoted bits further down).

   ...

   Some more data: It -looks- like what happens is that the
   flush_to_ldisc work queue entry constantly re-queues itself (because
   the PTY is full ?) and the workqueue thread will basically loop
   forver calling it without ever scheduling, thus starving the consumer
   process that could have emptied the PTY."

which is pretty much exactly the problem we fixed in a5660b41af.

Milton Miller pointed out the 'unsigned int' issue.

Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reported-by: Milton Miller <miltonm@bga.com>
Cc: Stefan Bigler <stefan.bigler@keymile.com>
Cc: Toby Gray <toby.gray@realvnc.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2011-06-04 06:33:24 +09:00
Felipe Balbi b1c43f82c5 tty: make receive_buf() return the amout of bytes received
it makes it simpler to keep track of the amount of
bytes received and simplifies how flush_to_ldisc counts
the remaining bytes. It also fixes a bug of lost bytes
on n_tty when flushing too many bytes via the USB
serial gadget driver.

Tested-by: Stefan Bigler <stefan.bigler@keymile.com>
Tested-by: Toby Gray <toby.gray@realvnc.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2011-04-22 17:31:53 -07:00
Linus Torvalds a5660b41af tty: fix endless work loop when the buffer fills up
Commit f23eb2b2b2 ('tty: stop using "delayed_work" in the tty layer')
ended up causing hung machines on UP with no preemption, because the
work routine to flip the buffer data to the ldisc would endlessly re-arm
itself if the destination buffer had filled up.

With the delayed work, that only caused a timer-driving polling of the
tty state every timer tick, but without the delay we just ended up with
basically a busy loop instead.

Stop the insane polling, and instead make the code that opens up the
receive room re-schedule the buffer flip work.  That's what we should
have been doing anyway.

This same "poll for tty room" issue is almost certainly also the cause
of excessive kworker activity when idle reported by Dave Jones, who also
reported "flush_to_ldisc executing 2500 times a second" back in Nov 2010:

  http://lkml.org/lkml/2010/11/30/592

which is that silly flushing done every timer tick.  Wasting both power
and CPU for no good reason.

Reported-and-tested-by: Alexander Beregalov <a.beregalov@gmail.com>
Reported-and-tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>
Cc: Greg KH <gregkh@suse.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Dave Jones <davej@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2011-04-04 14:26:54 -07:00
Greg Kroah-Hartman 96fd7ce58f TTY: create drivers/tty and move the tty core files there
The tty code should be in its own subdirectory and not in the char
driver with all of the cruft that is currently there.

Based on work done by Arnd Bergmann <arnd@arndb.de>

Acked-by: Arnd Bergmann <arnd@arndb.de>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2010-11-05 08:10:33 -07:00