linux-kernel - Re: [PATCH v7 2/2] x86 mce: use new printk recursion disabling interface

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1340630061.2507.61.camel@laptop>
Date:	Mon, 25 Jun 2012 15:14:21 +0200
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	ShuoX Liu <shuox.liu@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Borislav Petkov <bp@...en8.de>,
	Yanmin Zhang <yanmin_zhang@...ux.intel.com>,
	"Luck, Tony" <tony.luck@...el.com>,
	"andi@...stfloor.org" <andi@...stfloor.org>,
	Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v7 2/2] x86 mce: use new printk recursion disabling
 interface

On Mon, 2012-06-25 at 11:17 +0200, Ingo Molnar wrote:
> * ShuoX Liu <shuox.liu@...el.com> wrote:
> 
> > From: ShuoX Liu <shuox.liu@...el.com>
> > 
> > On x86 machines, some times MCE happens just when kernel calls printk
> > to output some log info to serial console, while usually MCE module in
> > kernel is used to print out some hardware error information, such like
> > bad cache or bad memory bank. That causes printk recursion and printk
> > would omit MCE printk output.
> > 
> > We hit it when running MTBF testing on Android ATOM mobiles.
> > 
> > Here in print_mce, we choose to disable printk recursion to make sure
> > MCE logs printed out.
> > 
> > Signed-off-by: Yanmin Zhang <yanmin_zhang@...ux.intel.com>
> > Signed-off-by: ShuoX Liu <shuox.liu@...el.com>
> > ---
> >  arch/x86/kernel/cpu/mcheck/mce.c |    6 +++++-
> >  1 files changed, 5 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> > index 2afcbd2..6056e94 100644
> > --- a/arch/x86/kernel/cpu/mcheck/mce.c
> > +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> > @@ -242,6 +242,7 @@ static void print_mce(struct mce *m)
> >  {
> >  	int ret = 0;
> >  
> > +	printk_recursion_check_disable();
> >  	pr_emerg(HW_ERR "CPU %d: Machine Check Exception: %Lx Bank %d: %016Lx\n",
> >  	       m->extcpu, m->mcgstatus, m->bank, m->status);
> >  
> > @@ -275,10 +276,13 @@ static void print_mce(struct mce *m)
> >  	 * (if the CPU has an implementation for that)
> >  	 */
> >  	ret = atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, m);
> > -	if (ret == NOTIFY_STOP)
> > +	if (ret == NOTIFY_STOP) {
> > +		printk_recursion_check_enable();
> >  		return;
> > +	}
> >  
> >  	pr_emerg_ratelimited(HW_ERR "Run the above through 'mcelog --ascii'\n");
> > +	printk_recursion_check_enable();
> 
> Ok, this looks useful and it solves a real problem, but I'd 
> prefer a better interface: instead of exposing the guts of 
> printk to drivers in an unsafe manner (and allowing them to keep 
> printk in an unsafe state indefinitely), shouldn't we instead 
> introduce printk_emergency() (and variants) that just disable 
> the recursion check, do the printk and then enable them?

I really don't see why you'd do anything like the above. For one the
oops_in_progress thing is exported, so you could simply use that, you're
going to panic the machine here anyway, right?

Furthermore the whole recursion stuff is broken anyway, see:
  http://marc.info/?l=linux-kernel&m=132446646923333

That also introduces recursive_printk() which you could (ab)use if
needed.

That said, its not entirely clear to me what context you're in when
you're calling this print_mce(), it looks like its only used from
mce_panic().

That already does bust_spinlocks() which already increments the
oops_in_progress thing, so WTF are you doing anyway?

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