Re: [2.6.33-rc5] tty: possible irq lock inversion dependency intty_fasync

[Linus Torvalds]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/