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]

Message-ID: <20160120145843.4a51359e@lxorguk.ukuu.org.uk>
Date:	Wed, 20 Jan 2016 14:58:43 +0000
From:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Dmitry Vyukov <dvyukov@...gle.com>, 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

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

Alan