Re: [PATCH V2] [tty] Fix possible race in n_tty_read()

[Stanislaw Gruszka <sgruszka@redhat.com>]

On Wed, Aug 08, 2012 at 04:27:25PM +0100, Alan Cox wrote:
> On Wed, 08 Aug 2012 16:28:47 +0200
> Stanislav Kozina <skozina@redhat.com> wrote:
> 
> > Fix possible panic caused by unlocked access to tty->read_cnt in 
> > while-loop condition in n_tty_read().
> 
> Should this also be removing the BUG_ON check you noted in the other
> email was not valid now ?

You talk about 
http://marc.info/?l=linux-serial&m=134318985920881&w=2

Is possible that we can call n_tty_read() after n_tty_close() ? How oterwise
tty->read_buf could become NULL?

If I understand correctly Stanislav's patch solve below race condtion:

CPU0					CPU1
n_tty_read:				reset_buffer_flags:

while (nr && tty->read_cnt) {

  					spin_lock_irqsave(&tty->read_lock, flags);
					tty->read_head = tty->read_tail = tty->read_cnt = 0;
  					spin_lock_irqsave(&tty->read_lock, flags);

  spin_lock_irqsave(&tty->read_lock, flags);
  
  tty->read_cnt--;

  spin_lock_irqsave(&tty->read_lock, flags);

  /* Now tty->read_cnt is negative */

}

what itself could have varsious nasty consequences, i.e. ininite
loop. Is also possible that negative tty->read_cnt would result in
tty->read_buf == NULL ? If so, I'm not quite understand that.

Stanislaw