linux-kernel - Re: [PATCH 2/4] tty: Hold tty_ldisc_lock() during tty

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1535553021.23560.50.camel@arista.com>
Date:   Wed, 29 Aug 2018 15:30:21 +0100
From:   Dmitry Safonov <dima@...sta.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.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,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCH 2/4] tty: Hold tty_ldisc_lock() during tty_reopen()

Hi Sergey,

On Wed, 2018-08-29 at 13:34 +0900, Sergey Senozhatsky wrote:
> Hi,
> 
> Cc-ing Benjamin on this.

Thanks!

> 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.

Probably, it's worth to know what exactly has he tried and what was the
backtrace he got in the result..
Hopefully, we'll hear more.

It might be also worth to review tty_ldisc_deinit(), I thought it's
safe to destroy ldisc there under tty lock during tty release, but may
be that is another non-safe place.

-- 
Thanks again,
             Dmitry