Re: [PATCH 2/2] vt: selection, push sel_lock up

[Greg KH]

On Fri, Feb 28, 2020 at 12:54:06PM +0100, Jiri Slaby wrote:
> sel_lock cannot nest in the console lock. Thanks to syzkaller, the
> kernel states firmly:
> 
> > WARNING: possible circular locking dependency detected
> > 5.6.0-rc3-syzkaller #0 Not tainted
> > ------------------------------------------------------
> > syz-executor.4/20336 is trying to acquire lock:
> > ffff8880a2e952a0 (&tty->termios_rwsem){++++}, at: tty_unthrottle+0x22/0x100 drivers/tty/tty_ioctl.c:136
> >
> > but task is already holding lock:
> > ffffffff89462e70 (sel_lock){+.+.}, at: paste_selection+0x118/0x470 drivers/tty/vt/selection.c:374
> >
> > which lock already depends on the new lock.
> >
> > the existing dependency chain (in reverse order) is:
> >
> > -> #2 (sel_lock){+.+.}:
> >        mutex_lock_nested+0x1b/0x30 kernel/locking/mutex.c:1118
> >        set_selection_kernel+0x3b8/0x18a0 drivers/tty/vt/selection.c:217
> >        set_selection_user+0x63/0x80 drivers/tty/vt/selection.c:181
> >        tioclinux+0x103/0x530 drivers/tty/vt/vt.c:3050
> >        vt_ioctl+0x3f1/0x3a30 drivers/tty/vt/vt_ioctl.c:364
> 
> This is ioctl(TIOCL_SETSEL).
> Locks held on the path: console_lock -> sel_lock
> 
> > -> #1 (console_lock){+.+.}:
> >        console_lock+0x46/0x70 kernel/printk/printk.c:2289
> >        con_flush_chars+0x50/0x650 drivers/tty/vt/vt.c:3223
> >        n_tty_write+0xeae/0x1200 drivers/tty/n_tty.c:2350
> >        do_tty_write drivers/tty/tty_io.c:962 [inline]
> >        tty_write+0x5a1/0x950 drivers/tty/tty_io.c:1046
> 
> This is write().
> Locks held on the path: termios_rwsem -> console_lock
> 
> > -> #0 (&tty->termios_rwsem){++++}:
> >        down_write+0x57/0x140 kernel/locking/rwsem.c:1534
> >        tty_unthrottle+0x22/0x100 drivers/tty/tty_ioctl.c:136
> >        mkiss_receive_buf+0x12aa/0x1340 drivers/net/hamradio/mkiss.c:902
> >        tty_ldisc_receive_buf+0x12f/0x170 drivers/tty/tty_buffer.c:465
> >        paste_selection+0x346/0x470 drivers/tty/vt/selection.c:389
> >        tioclinux+0x121/0x530 drivers/tty/vt/vt.c:3055
> >        vt_ioctl+0x3f1/0x3a30 drivers/tty/vt/vt_ioctl.c:364
> 
> This is ioctl(TIOCL_PASTESEL).
> Locks held on the path: sel_lock -> termios_rwsem
> 
> > other info that might help us debug this:
> >
> > Chain exists of:
> >   &tty->termios_rwsem --> console_lock --> sel_lock
> 
> Clearly. From the above, we have:
>  console_lock -> sel_lock
>  sel_lock -> termios_rwsem
>  termios_rwsem -> console_lock
> 
> Fix this by reversing the console_lock -> sel_lock dependency in
> ioctl(TIOCL_SETSEL). First, lock sel_lock, then console_lock.
> 
> Signed-off-by: Jiri Slaby <jslaby@xxxxxxx>
> Reported-by: syzbot+26183d9746e62da329b8@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 07e6124a1a46 ("vt: selection, close sel_buffer race")

As 07e6124a1a46 was marked for stable, both of these should be as well,
right?

And did you happen to test these two with the syzbot tool to see if it
really did fix the report?

thanks,

greg k-h