Skip to content
  • 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