linux-kernel - Re: [PATCH] printk deadlocks if called with runqueue lock held

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.1.00.0801180918110.2957@woody.linux-foundation.org>
Date:	Fri, 18 Jan 2008 09:30:20 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Jan Kiszka <jan.kiszka@...mens.com>
cc:	Steven Rostedt <rostedt@...dmis.org>, Jiri Kosina <jikos@...os.cz>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Alexey Dobriyan <adobriyan@...nvz.org>,
	Kirill Korotaev <dev@...ru>, Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] printk deadlocks if called with runqueue lock held

On Fri, 18 Jan 2008, Jan Kiszka wrote:
> Steven Rostedt wrote:
> ...
> > @@ -978,7 +980,13 @@ void release_console_sem(void)
> >  	console_locked = 0;
> >  	up(&console_sem);
> 
> Hmm, just looking at this fragment: Doesn't up() include the risk of
> running onto the runqueue lock as well?

In theory yes.

I suspect it would never ever be a problem in practice (the case we care 
about is running with interrupts disabled, and we got it with 
down_trylock()), so as this is only about essentially custon debug or oops 
things anyway, it's probably not worth fixing.

That said, you're definitely right in theory.

But *IF* we want to fix the almost certainly purely theoretical problem, 
it would be possible but fairly ugly.

We'd need to (a) make it a mutex rather than a semaphore (which is 
definitely not the ugly part), and then - the ugly part - (b) expose a 
whole new mutex interface: an enhanched version of "mutex_trylock()" that 
*also* keeps the mutex spinlock locked, and then instead of using 
"mutex_unlock()" we'd use a special "mutex_unlock_atomic()" that knows the 
spinlock was held over the whole time.

So it would then use something like

	if (mutex_trylock_atomic(..)) {

		mutex_unlock_atomic(..);
	}

and that would work out ok.

It's likely not that hard, and in fact this may be why "console_sem" was 
never converted to a mutex: I think Ingo tried, but it didn't work right 
with debugging enabled, and I can well imagine that it was all due to this 
issue. But if we do those _atomic() versions, we'd probably fix that 
problem.

So maybe the "ugly new interface" would actually be a cleanup in the long 
run, by possibly fixing the fact that things just *happened* to work with 
semaphores because they didn't do the fancy debug version..

Ingo?

			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/