Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2] tty: Fix data race between tiocsti() and flush_to_ldisc() | From | Jiri Slaby <> | Date | Mon, 23 Aug 2021 10:52:57 +0200 |
| |
On 23. 08. 21, 2:06, Nguyen Dinh Phi wrote: > The ops->receive_buf() may be accessed concurrently from these two > functions. If the driver flushes data to the line discipline > receive_buf() method while tiocsti() is waiting for the > ops->receive_buf() to finish its work, the data race will happen. > > For example: > tty_ioctl |tty_ldisc_receive_buf > ->tioctsi | ->tty_port_default_receive_buf > | ->tty_ldisc_receive_buf > ->hci_uart_tty_receive | ->hci_uart_tty_receive > ->h4_recv | ->h4_recv > > In this case, the h4 receive buffer will be overwritten by the > latecomer, and we will lost the data. > > Hence, change tioctsi() function to use the exclusive lock interface > from tty_buffer to avoid the data race.
This makes sense. It mimics paste_selection() -- the lock order is preexisting. The normal path processing has the locks inverted, not sure why it doesn't matter :): flush_to_ldisc mutex_lock(&buf->lock); <--------- A (mutex) receive_buf() tty_port_default_receive_buf() disc = tty_ldisc_ref(tty); <----- B (ldsem for READ)
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
> Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com> > Reported-by: syzbot+97388eb9d31b997fe1d0@syzkaller.appspotmail.com > --- > V2: > - Remove FIXME comment. > > drivers/tty/tty_io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c > index e8532006e960..6616d4a0d41d 100644 > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -2290,8 +2290,6 @@ static int tty_fasync(int fd, struct file *filp, int on) > * Locking: > * Called functions take tty_ldiscs_lock > * current->signal->tty check is safe without locks > - * > - * FIXME: may race normal receive processing > */ > > static int tiocsti(struct tty_struct *tty, char __user *p) > @@ -2307,8 +2305,10 @@ static int tiocsti(struct tty_struct *tty, char __user *p) > ld = tty_ldisc_ref_wait(tty);
Here (and in paste_selection()), it is lock "B (ldsem) for READ".
> if (!ld) > return -EIO; > + tty_buffer_lock_exclusive(tty->port);
and "A (mutex)".
> if (ld->ops->receive_buf) > ld->ops->receive_buf(tty, &ch, &mbz, 1); > + tty_buffer_unlock_exclusive(tty->port); > tty_ldisc_deref(ld); > return 0; > }
thanks, -- js suse labs
| |