Skip to content
Snippets Groups Projects
  1. May 04, 2014
  2. Apr 25, 2014
    • Manfred Schlaegl's avatar
      tty: Fix race condition between __tty_buffer_request_room and flush_to_ldisc · 6a20dbd6
      Manfred Schlaegl authored
      
      The race was introduced while development of linux-3.11 by
      e8437d7e and
      e9975fde.
      Originally it was found and reproduced on linux-3.12.15 and
      linux-3.12.15-rt25, by sending 500 byte blocks with 115kbaud to the
      target uart in a loop with 100 milliseconds delay.
      
      In short:
       1. The consumer flush_to_ldisc is on to remove the head tty_buffer.
       2. The producer adds a number of bytes, so that a new tty_buffer must
      	be allocated and added by __tty_buffer_request_room.
       3. The consumer removes the head tty_buffer element, without handling
      	newly committed data.
      
      Detailed example:
       * Initial buffer:
         * Head, Tail -> 0: used=250; commit=250; read=240; next=NULL
       * Consumer: ''flush_to_ldisc''
         * consumed 10 Byte
         * buffer:
           * Head, Tail -> 0: used=250; commit=250; read=250; next=NULL
      {{{
      		count = head->commit - head->read;	// count = 0
      		if (!count) {				// enter
      			// INTERRUPTED BY PRODUCER ->
      			if (head->next == NULL)
      				break;
      			buf->head = head->next;
      			tty_buffer_free(port, head);
      			continue;
      		}
      }}}
       * Producer: tty_insert_flip_... 10 bytes + tty_flip_buffer_push
         * buffer:
           * Head, Tail -> 0: used=250; commit=250; read=250; next=NULL
         * added 6 bytes: head-element filled to maximum.
           * buffer:
             * Head, Tail -> 0: used=256; commit=250; read=250; next=NULL
         * added 4 bytes: __tty_buffer_request_room is called
           * buffer:
             * Head -> 0: used=256; commit=256; read=250; next=1
             * Tail -> 1: used=4; commit=0; read=250 next=NULL
         * push (tty_flip_buffer_push)
           * buffer:
             * Head -> 0: used=256; commit=256; read=250; next=1
             * Tail -> 1: used=4; commit=4; read=250 next=NULL
       * Consumer
      {{{
      		count = head->commit - head->read;
      		if (!count) {
      			// INTERRUPTED BY PRODUCER <-
      			if (head->next == NULL)		// -> no break
      				break;
      			buf->head = head->next;
      			tty_buffer_free(port, head);
      			// ERROR: tty_buffer head freed -> 6 bytes lost
      			continue;
      		}
      }}}
      
      This patch reintroduces a spin_lock to protect this case. Perhaps later
      a lock-less solution could be found.
      
      Signed-off-by: default avatarManfred Schlaegl <manfred.schlaegl@gmx.at>
      Cc: stable <stable@vger.kernel.org> # 3.11
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      6a20dbd6
  3. Mar 01, 2014
    • Peter Hurley's avatar
      tty: Fix low_latency BUG · a9c3f68f
      Peter Hurley authored
      The user-settable knob, low_latency, has been the source of
      several BUG reports which stem from flush_to_ldisc() running
      in interrupt context. Since 3.12, which added several sleeping
      locks (termios_rwsem and buf->lock) to the input processing path,
      the frequency of these BUG reports has increased.
      
      Note that changes in 3.12 did not introduce this regression;
      sleeping locks were first added to the input processing path
      with the removal of the BKL from N_TTY in commit
      a88a69c9,
      'n_tty: Fix loss of echoed characters and remove bkl from n_tty'
      and later in commit 38db8979,
      'tty: throttling race fix'. Since those changes, executing
      flush_to_ldisc() in interrupt_context (ie, low_latency set), is unsafe.
      
      However, since most devices do not validate if the low_latency
      setting is appropriate for the context (process or interrupt) in
      which they receive data, some reports are due to misconfiguration.
      Further, serial dma devices for which dma fails, resort to
      interrupt receiving as a backup without resetting low_latency.
      
      Historically, low_latency was used to force wake-up the reading
      process rather than wait for the next scheduler tick. The
      effect was to trim multiple milliseconds of latency from
      when the process would receive new data.
      
      Recent tests [1] have shown that the reading process now receives
      data with only 10's of microseconds latency without low_latency set.
      
      Remove the low_latency rx steering from tty_flip_buffer_push();
      however, leave the knob as an optional hint to drivers that can
      tune their rx fifos and such like. Cleanup stale code comments
      regarding low_latency.
      
      [1] https://lkml.org/lkml/2014/2/20/434
      
      
      
      "Yay.. thats an annoying historical pain in the butt gone."
      	-- Alan Cox
      
      Reported-by: default avatarBeat Bolli <bbolli@ewanet.ch>
      Reported-by: default avatarPavel Roskin <proski@gnu.org>
      Acked-by: default avatarDavid Sterba <dsterba@suse.cz>
      Cc: Grant Edwards <grant.b.edwards@gmail.com>
      Cc: Stanislaw Gruszka <sgruszka@redhat.com>
      Cc: Hal Murray <murray+fedora@ip-64-139-1-69.sjc.megapath.net>
      Cc: <stable@vger.kernel.org> # 3.12.x+
      Signed-off-by: default avatarPeter Hurley <peter@hurleysoftware.com>
      Signed-off-by: default avatarAlan Cox <alan@linux.intel.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      a9c3f68f
  4. Jan 08, 2014
  5. Dec 09, 2013
  6. Jul 24, 2013
  7. Apr 10, 2013
    • Peter Hurley's avatar
      tty: Fix race condition if flushing tty flip buffers · 39f610e4
      Peter Hurley authored
      
      As Ilya Zykov identified in his patch 'PROBLEM: Race condition in
      tty buffer's function flush_to_ldisc()', a race condition exists
      which allows a parallel flush_to_ldisc() to flush and free the tty
      flip buffers while those buffers are in-use. For example,
      
        CPU 0                         |  CPU 1                                  |  CPU 2
                                      | flush_to_ldisc()                        |
                                      |  grab spin lock                         |
      tty_buffer_flush()              |                                         | flush_to_ldisc()
       wait for spin lock             |                                         |  wait for spin lock
                                      |   if (!test_and_set_bit(TTYP_FLUSHING)) |
                                      |    while (next flip buffer)             |
                                      |     ...                                 |
                                      |     drop spin lock                      |
        grab spin lock                |                                         |
         if (test_bit(TTYP_FLUSHING)) |                                         |
          set_bit(TTYP_FLUSHPENDING)  |      receive_buf()                      |
          drop spin lock              |                                         |
                                      |                                         |   grab spin lock
                                      |                                         |    if (!test_and_set_bit(TTYP_FLUSHING))
                                      |                                         |    if (test_bit(TTYP_FLUSHPENDING))
                                      |                                         |    __tty_buffer_flush()
      
      CPU 2 has just flushed and freed all tty flip buffers while CPU 1 is
      transferring data from the head flip buffer.
      
      The original patch was rejected under the assumption that parallel
      flush_to_ldisc() was not possible. Because of necessary changes to
      the workqueue api, work items can execute in parallel on SMP.
      
      This patch differs slightly from the original patch by testing for
      a pending flush _after_ each receive_buf(), since TTYP_FLUSHPENDING
      can only be set while the lock is dropped around receive_buf().
      
      Reported-by: default avatarIlya Zykov <linux@izyk.ru>
      Signed-off-by: default avatarPeter Hurley <peter@hurleysoftware.com>
      Acked-by: default avatarIlya Zykov <linux@izyk.ru>
      Cc: Jiri Slaby <jslaby@suse.cz>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      39f610e4
  8. Mar 04, 2013
  9. Feb 13, 2013
    • George Spelvin's avatar
      pps: Move timestamp read into PPS code proper · 593fb1ae
      George Spelvin authored
      
      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: default avatarPeter Hurley <peter@hurleysoftware.com>
      Signed-off-by: default avatarGeorge Spelvin <linux@horizon.com>
      Acked-by: default avatarRodolfo Giometti <giometti@enneenne.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      593fb1ae
  10. Jan 21, 2013
    • Ilya Zykov's avatar
      tty: Correct tty buffer flush. · 64325a3b
      Ilya Zykov authored
      
        The root of problem is carelessly zeroing pointer(in function __tty_buffer_flush()),
      when another thread can use it. It can be cause of "NULL pointer dereference".
        Main idea of the patch, this is never free last (struct tty_buffer) in the active buffer.
      Only flush the data for ldisc(buf->head->read = buf->head->commit).
      At that moment driver can collect(write) data in buffer without conflict.
      It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.
      
      Also revert:
        commit c56a00a1
        tty: hold lock across tty buffer finding and buffer filling
      In order to delete the unneeded locks any more.
      
      Signed-off-by: default avatarIlya Zykov <ilya@ilyx.ru>
      CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      64325a3b
  11. Jan 16, 2013
  12. Oct 25, 2012
  13. Oct 24, 2012
  14. Oct 23, 2012
Loading