Commit Graph

20 Commits

Author SHA1 Message Date
Peter Hurley d8c1f929aa tty: Only guarantee termios read safety for throttle/unthrottle
No tty driver modifies termios during throttle() or unthrottle().
Therefore, only read safety is required.

However, tty_throttle_safe and tty_unthrottle_safe must still be
mutually exclusive; introduce throttle_mutex for that purpose.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:43:02 -07:00
Peter Hurley 6a1c0680cf tty: Convert termios_mutex to termios_rwsem
termios is commonly accessed unsafely (especially by N_TTY)
because the existing mutex forces exclusive access.
Convert existing usage.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-07-23 16:43:01 -07:00
Peter Hurley 579a00a5c2 tty: Fix unsafe bit ops in tty_throttle_safe/unthrottle_safe
tty->flags needs to be atomically modified.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-04-15 11:08:12 -07:00
Peter Hurley e7f3880cd9 tty: Fix recursive deadlock in tty_perform_flush()
tty_perform_flush() can deadlock when called while holding
a line discipline reference. By definition, all ldisc drivers
hold a ldisc reference, so calls originating from ldisc drivers
must not block for a ldisc reference.

The deadlock can occur when:
  CPU 0                    |  CPU 1
                           |
tty_ldisc_ref(tty)         |
....                       | <line discipline halted>
tty_ldisc_ref_wait(tty)    |
                           |

CPU 0 cannot progess because it cannot obtain an ldisc reference
with the line discipline has been halted (thus no new references
are granted).
CPU 1 cannot progress because an outstanding ldisc reference
has not been released.

An in-tree call-tree audit of tty_perform_flush() [1] shows 5
ldisc drivers calling tty_perform_flush() indirectly via
n_tty_ioctl_helper() and 2 ldisc drivers calling directly.
A single tty driver safely uses the function.

[1]
Recursive usage:

/* These functions are line discipline ioctls and thus
 * recursive wrt line discipline references */

tty_perform_flush() - ./drivers/tty/tty_ioctl.c
    n_tty_ioctl_helper()
        hci_uart_tty_ioctl(default) - drivers/bluetooth/hci_ldisc.c (N_HCI)
        n_hdlc_tty_ioctl(default) - drivers/tty/n_hdlc.c (N_HDLC)
        gsmld_ioctl(default) - drivers/tty/n_gsm.c (N_GSM0710)
        n_tty_ioctl(default) - drivers/tty/n_tty.c (N_TTY)
        gigaset_tty_ioctl(default) - drivers/isdn/gigaset/ser-gigaset.c (N_GIGASET_M101)
    ppp_synctty_ioctl(TCFLSH) - drivers/net/ppp/pps_synctty.c
    ppp_asynctty_ioctl(TCFLSH) - drivers/net/ppp/ppp_async.c

Non-recursive use:

tty_perform_flush() - drivers/tty/tty_ioctl.c
    ipw_ioctl(TCFLSH) - drivers/tty/ipwireless/tty.c
       /* This function is a tty i/o ioctl method, which
        * is invoked by tty_ioctl() */

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:52:24 -07:00
Jiri Slaby 6865ff222c TTY: do not warn about setting speed via SPD_*
The warning is there since 2.1.69 and we have not seen anybody
reporting it in the past decade. Remove the warning now.

tty_get_baud_rate can now be inline. This gives us one less
EXPORT_SYMBOL.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-18 16:19:05 -07:00
Peter Hurley 70bc126471 tty: Add safe tty throttle/unthrottle functions
The tty driver can become stuck throttled due to race conditions
between throttle and unthrottle, when the decision to throttle
or unthrottle is conditional. The following example helps to
illustrate the race:

  CPU 0                        |  CPU 1
                               |
if (condition A)               |
                               | <processing such that A not true>
                               | if (!condition A)
                               |     unthrottle()
    throttle()                 |
                               |

Note the converse is also possible; ie.,

  CPU 0                        |  CPU 1
                               |
                               | if (!condition A)
<processing such that A true>  |
if (condition A)               |
    throttle()                 |
                               |     unthrottle()
                               |

Add new throttle/unthrottle functions based on the familiar model
of task state and schedule/wake. For example,

    while (1) {
        tty_set_flow_change(tty, TTY_THROTTLE_SAFE);
        if (!condition)
            break;
        if (!tty_throttle_safe(tty))
            break;
    }
    __tty_set_flow_change(tty, 0);

In this example, if an unthrottle occurs after the condition is
evaluated but before tty_throttle_safe(), then tty_throttle_safe()
will return non-zero, looping and forcing the re-evaluation of
condition.

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
Oleg Nesterov 183d95cdd8 tty: set_termios/set_termiox should not return -EINTR
See https://bugzilla.redhat.com/show_bug.cgi?id=904907
read command causes bash to abort with double free or corruption (out).

A simple test-case from Roman:

	// Compile the reproducer and send sigchld ti that process.
	// EINTR occurs even if SA_RESTART flag is set.

	void handler(int sig)
	{
	}

	main()
	{
	  struct sigaction act;
	  act.sa_handler = handler;
	  act.sa_flags = SA_RESTART;
	  sigaction (SIGCHLD, &act, 0);
	  struct termio ttp;
	  ioctl(0, TCGETA, &ttp);
	  while(1)
	  {
	    if (ioctl(0, TCSETAW, ttp) < 0)
	      {
		if (errno == EINTR)
		{
		  fprintf(stderr, "BUG!"); return(1);
		}
	      }
	  }
	}

Change set_termios/set_termiox to return -ERESTARTSYS to fix this
particular problem.

I didn't dare to change other EINTR's in drivers/tty/, but they look
equally wrong.

Reported-by: Roman Rakus <rrakus@redhat.com>
Reported-by: Lingzhu Xiang <lxiang@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-02-04 15:02:14 -08:00
Ilya Zykov a1bf958442 tty: Add driver unthrottle in ioctl(...,TCFLSH,..).
Regression 'tty: fix "IRQ45: nobody cared"'
Regression commit 7b292b4bf9

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..).
At the time of request we can have full buffers and throttled driver too.
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and
there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers wake up.

Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-01-18 16:07:34 -08:00
Cyrill Gorcunov 06026d911c tty: pty - Move TIOCPKT handling into pty.c
Since this ioctl is for pty devices only move it to pty.c.

v2:
 - drop PTY_TYPE_MASTER test since it's master peer
   ioctl anyway (by jslaby@)

Suggested-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-10-25 12:07:18 -07:00
Alan Cox 9833facf90 tty: Fix up PPC fallout from the termios move
This fixes up the problem Stephen Rothwell reported when trying to merge -next

Signed-off-by: Alan Cox <alan@linux.intel.com>
Reported-by: Stephen Rothwell <sfr@cab.auug.org.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-07-17 09:10:55 -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
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
Thomas Meyer 8193c42906 tty: Support compat_ioctl get/set termios_locked
When running a Fedora 15 (x86) on an x86_64 kernel, in the boot process
plymouthd complains about those two missing ioctls:
[    2.581783] ioctl32(plymouthd:186): Unknown cmd fd(10) cmd(00005457){t:'T';sz:0} arg(ffb6a5d0) on /dev/tty1
[    2.581803] ioctl32(plymouthd:186): Unknown cmd fd(10) cmd(00005456){t:'T';sz:0} arg(ffb6a680) on /dev/tty1

both ioctl functions work on the 'struct termios' resp. 'struct termios2',
which has the same size (36 bytes resp. 44 bytes) on x86 and x86_64,
so it's just a matter of converting the pointer from userland.

Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Andrew Morton <akpm@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2011-10-18 14:17:11 -07:00
Linus Torvalds 99dff58562 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: (48 commits)
  serial: 8250_pci: add support for Cronyx Omega PCI multiserial board.
  tty/serial: Fix break handling for PORT_TEGRA
  tty/serial: Add explicit PORT_TEGRA type
  n_tracerouter and n_tracesink ldisc additions.
  Intel PTI implementaiton of MIPI 1149.7.
  Kernel documentation for the PTI feature.
  export kernel call get_task_comm().
  tty: Remove to support serial for S5P6442
  pch_phub: Support new device ML7223
  8250_pci: Add support for the Digi/IBM PCIe 2-port Adapter
  ASoC: Update cx20442 for TTY API change
  pch_uart: Support new device ML7223 IOH
  parport: Use request_muxed_region for IT87 probe and lock
  tty/serial: add support for Xilinx PS UART
  n_gsm: Use print_hex_dump_bytes
  drivers/tty/moxa.c: Put correct tty value
  TTY: tty_io, annotate locking functions
  TTY: serial_core, remove superfluous set_task_state
  TTY: serial_core, remove invalid test
  Char: moxa, fix locking in moxa_write
  ...

Fix up trivial conflicts in drivers/bluetooth/hci_ldisc.c and
drivers/tty/serial/Makefile.

I did the hci_ldisc thing as an evil merge, cleaning things up.
2011-05-23 12:23:20 -07:00
Jiri Kosina 07f9479a40 Merge branch 'master' into for-next
Fast-forwarded to current state of Linus' tree as there are patches to be
applied for files that didn't exist on the old branch.
2011-04-26 10:22:59 +02:00
Jovi Zhang 99edb3d10a tty: remove invalid location line in file header
remove invalid location line in each file header after location
moved from driver/char to driver/tty

Signed-off-by: Jovi Zhang <bookjovi@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2011-04-19 16:33:36 -07:00
Uwe Kleine-König f98e5b80ad tty: fix typo in descripton of tty_termios_encode_baud_rate
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2011-04-12 13:25:33 +02:00
Lucas De Marchi 25985edced Fix common misspellings
Fixes generated by 'codespell' and manually reviewed.

Signed-off-by: Lucas De Marchi <lucas.demarchi@profusion.mobi>
2011-03-31 11:26:23 -03:00
Alan Cox 8d075b199b tty: add a helper for setting termios data from kernel side
This basically encapsulates the small bit of locking knowledge needed. While
we are at it make sure we blow up on any more abusers and unsafe misuses of
ioctl for this kind of stuff.

We change the function to return an argument as at some point it needs to
honour the POSIX 'I asked for changes but got none of them' error reporting
corner case.

Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2011-02-17 12:03:52 -08: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