linux-kernel - Re: NMI watchdog dump does not print on hard lockup

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:   Tue, 17 Oct 2017 16:50:15 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Petr Mladek <pmladek@...e.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: NMI watchdog dump does not print on hard lockup

On (10/16/17 10:15), Steven Rostedt wrote:
[..]
> > just "brainstorming" it... with some silly ideas.
> > 
> > pushing the data from NMI panic might look like we are replacing one
> > deadlock scenario with another deadlock scenario. some of the console
> > drivers are soooo complex internally. so I have been thinking about...
> > may be we can extend struct console and add ->write_on_panic() and that
> > handler must be as lockless as possible; so lockless that calling it
> > from anything that is not panic() is a severe bug.
> 
> This may not be a bad idea. And make it so it can't be called unless we
> are in panic mode (or at least "oops in progress").

right.

we used to have that zap_locks() function, which used to re-init printk()
internal locks on panic (printk recursion while in panic, to be exact):
logbuf spin_lock and console_sem. I wasn't to fond of this function, it
was missing the fact that on panic every printk() is a direct printk (at
least we have such expectation), IOW, it involves
	console_unlock()->call_console_drivers()

so punching printk()'s locks and leaving console drivers' locks intact
was not fair. at all. so, to improve the situation, I removed zap_locks().
/* kidding */

we have sort of re-entrant printk() now. but not completely re-entrant,
because console drivers are not re-entrant. so we can do

a) add ->zap_locks() callback to console drivers

   each console (which wants to be useful) can re-init its locks there, we
   will call it from panic() only. but, given how complex some of the
   consoles, I'd much rather prefer

b) add ->write_on_panic() callback to console drivers

   and do a barely legal print out there

I don't expect/want/push for/etc every console driver to implement
->write_on_panic() callback, just several most commonly used ones.
basically, the ones that you and PeterZ are using.

we also can split our flush_on_panic() and factor out the most
important part of console_unlock(). the first flush_on_panic(), let's
call it flush_on_panic_immediately() or whatever we name it, can push
messages only to those console drivers that have ->write_on_panic()
enabled. and it must call factored out part of console_unlock(). we
don't want flush_on_panic_immediately() to attempt up() the console
semaphore, because this can deadlock. so that factored out __console_unlock()
won't care about console_sem at all.

the second flush_on_panic() can push the data to all registered and
enabled consoles. this has chances to deadlock, but we can be less
nervous about it [given that there was at least one console with
->write_on_panic()].

> If oops_in_progress is set, and the console has a "write_on_panic"
> handler, then just call that.

yes. I don't like oops_in_progress variable, but some flag is
definitely needed.

> Heck, if it doesn't have one, and early_printk is defined, then perhaps
> that should be the default "write_on_panic" output?

yes, early_printk is a good addition. my systems have
"# CONFIG_EARLY_PRINTK is not set".

	-ss