linux-kernel - Re: [2.6.33-rc5] tty: possible irq lock inversion dependency in 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: <alpine.LFD.2.00.1002062246390.3829@localhost.localdomain>
Date:	Sat, 6 Feb 2010 22:59:56 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Américo Wang <xiyou.wangcong@...il.com>
cc:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>, gregkh@...e.de,
	taviso@...gle.com, viro@...IV.linux.org.uk,
	linux-kernel@...r.kernel.org, ebiederm@...ssion.com,
	alan@...rguk.ukuu.org.uk, jdike@...toit.com, jln@...gle.com,
	mpm@...enic.com
Subject: Re: [2.6.33-rc5] tty: possible irq lock inversion dependency in
 tty_fasync

On Sun, 7 Feb 2010, Américo Wang wrote:
> 
> We already fixed this, a better fix:

No we didn't.

> http://lkml.org/lkml/2010/1/26/338
> 
> I sent a same fix with Greg's.

We already did that. You didn't read Tetsuo's email carefully.

Let's quote the important parts:

  "is not yet fixed as of 2.6.33-rc7."

and also the _second_ lockdep complaint he quotes, which starts out with

  [   81.651199] =========================================================
  [   81.651199] [ INFO: possible irq lock inversion dependency detected ]
  [   81.651199] 2.6.33-rc7 #11
  [   81.651199] ---------------------------------------------------------

(note the -rc7 there).

The problem? Look at f_getown: it does

	read_lock(&filp->f_owner.lock);

ie it holds f_owner without interrupts disabled. Now an interrupt comes 
in, and takes 'siglock' because it ends up sending a signal (timer, SIGIO, 
whatever). So you have a f_owner -> siglock ordering.

But we _also_ have a siglock -> ctrl_lock -> f_owner ordering, in that 
problematic tty_fasync() thing. So we have a ABBA deadlock situation.

Yes, it's hard (practically impossible) to trigger, because you have to 
get an interrupt just at the right point with all the right processes, but 
lockdep seems to be entirely correct. 

So it is simply _wrong_ to take f_owner while we hold ctrl_lock.

Which is why I suggest just reverting both the original problematic commit 
_and_ the commit you point to, and just fix the race with that pid_get/put 
pair instead. As per my patch.

		Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/