linux-kernel - Re: tty: deadlock between n_tracerouter_receivebuf and flush_to

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]

Date:	Wed, 20 Jan 2016 16:16:52 +0100
From:	Dmitry Vyukov <dvyukov@...gle.com>
To:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.com>,
	LKML <linux-kernel@...r.kernel.org>,
	J Freyensee <james_p_freyensee@...ux.intel.com>,
	syzkaller <syzkaller@...glegroups.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Eric Dumazet <edumazet@...gle.com>
Subject: Re: tty: deadlock between n_tracerouter_receivebuf and flush_to_ldisc

On Wed, Jan 20, 2016 at 3:58 PM, One Thousand Gnomes
<gnomes@...rguk.ukuu.org.uk> wrote:
>> I read that, I didn't understand it. Which link is wrong and why?
>>
>> > And I don't understand how the following is a deadlock, since there is
>> > no cycle...
>> >
>> >  Possible unsafe locking scenario:
>> >       CPU0                    CPU1
>> >        ----                    ----
>> >   lock(&buf->lock);
>> >                                lock(&o_tty->termios_rwsem/1);
>> >                                lock(&buf->lock);
>> >   lock(routelock);
>>
>> Ignore the stupid picture, it only really works for simple cases.
>
> There are two line disciplines using two different locking orders
>
> The two line disciplines never execute at once. A given tty is either
> using one or the other and there is a clear and correctly locked
> changeover.
>
>
> semantically its something a bit like
>
>
>  foo(x)
>  {
>           if (x == 1) {
>                 lock(A)
>                 lock(B)
>           } else {
>                 lock(B)
>                 lock(A)
>           }
>
>           Do stuff();
>
>           if (x == 1) {
>                 unlock(B)
>                 unlock(A)
>           } else {
>                 unlock(A)
>                 unlock(B)
>           }
> }
>
> with the guarantee made elsewhere that no instances of foo(1) and foo(0)
> are ever executing at the same time.
>
> That's not by dumb design - it's an interesting "nobody ever noticed
> this" turned up by the lock detector between two totaly unrelated bits of
> code.

In out user-space deadlock detector we have an annotation along the
lines of "forget all info this particular mutex" for such cases
(between foo(0) and foo(1)). Is there something similar in lockdep?