lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 30 Aug 2018 15:16:55 +1000 From: Benjamin Herrenschmidt <benh@...nel.crashing.org> To: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>, Dmitry Safonov <dima@...sta.com> Cc: linux-kernel@...r.kernel.org, Dmitry Safonov <0x7f454c46@...il.com>, Daniel Axtens <dja@...ens.net>, Dmitry Vyukov <dvyukov@...gle.com>, Michael Neuling <mikey@...ling.org>, Mikulas Patocka <mpatocka@...hat.com>, Pasi Kärkkäinen <pasik@....fi>, Peter Hurley <peter@...leysoftware.com>, Tan Xiaojun <tanxiaojun@...wei.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby <jslaby@...e.com>, stable@...r.kernel.org Subject: Re: [PATCH 2/4] tty: Hold tty_ldisc_lock() during tty_reopen() On Wed, 2018-08-29 at 13:34 +0900, Sergey Senozhatsky wrote: > Hi, > > Cc-ing Benjamin on this. > > On (08/29/18 03:23), Dmitry Safonov wrote: > > BUG: unable to handle kernel paging request at 0000000000002260 > > IP: [..] n_tty_receive_buf_common+0x5f/0x86d > > Workqueue: events_unbound flush_to_ldisc > > Call Trace: > > [..] n_tty_receive_buf2 > > [..] tty_ldisc_receive_buf > > [..] flush_to_ldisc > > [..] process_one_work > > [..] worker_thread > > [..] kthread > > [..] ret_from_fork > > Seems that you are not the first one to hit this NULL deref. > > > I think, tty_ldisc_reinit() should be called with ldisc_sem hold for > > writing, which will protect any reader against line discipline changes. > > Per https://lore.kernel.org/patchwork/patch/777220/ > > : Note that we noticed one path that called reinit without the ldisc lock > : held for writing, we added that, but it didn't fix the problem. > > And I guess that Ben meant the same reinit path which you patched: This is too old for me to remember buit yes, there definitely was a bug there... > > @@ -1267,15 +1267,20 @@ static int tty_reopen(struct tty_struct *tty) > > if (test_bit(TTY_EXCLUSIVE, &tty->flags) && !capable(CAP_SYS_ADMIN)) > > return -EBUSY; > > > > - tty->count++; > > + retval = tty_ldisc_lock(tty, 5 * HZ); > > + if (retval) > > + return retval; > > > > + tty->count++; > > if (tty->ldisc) > > - return 0; > > + goto out_unlock; > > > > retval = tty_ldisc_reinit(tty, tty->termios.c_line); > > if (retval) > > tty->count--; > > > > +out_unlock: > > + tty_ldisc_unlock(tty); > > return retval; > > } > > -ss
Powered by blists - more mailing lists