linux-kernel - Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup

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:	Mon, 4 Apr 2016 13:52:06 +0200
From:	Jan Kara <jack@...e.cz>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	Borislav Petkov <bp@...en8.de>, Andy Lutomirski <luto@...nel.org>,
	X86 ML <x86@...nel.org>, Paolo Bonzini <pbonzini@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	KVM list <kvm@...r.kernel.org>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	xen-devel <Xen-devel@...ts.xen.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jan Kara <jack@...e.cz>, pmladek@...e.cz
Subject: Re: [PATCH v5 3/9] x86/head: Move early exception panic code into
 early_fixup_exception

On Sat 02-04-16 13:58:19, Andy Lutomirski wrote:
> [cc Jan Kara]
> 
> On Sat, Apr 2, 2016 at 1:47 PM, Borislav Petkov <bp@...en8.de> wrote:
> > On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
> >> Given that I this isn't really a regression with my patches (it
> >> probably never worked much better on 32-bit and the regs never would
> >> have shown at all on 64-bit),
> >
> > You're right. That thing calls printk *and* early_printk, WTF:
> >
> > #ifdef CONFIG_EARLY_PRINTK
> >
> >         call early_printk
> >         ...
> >
> >         call dump_stack
> >
> >         ...
> >
> >         call __print_symbol
> >
> > those last two call printk. Great.
> >
> >> I propose a different approach: make
> >> printk work earlier.  Something like:
> >>
> >> if (early) {
> >>     early_printk(args);
> >> }
> >>
> >> or early_vprintk or whatever.
> >>
> >> If the cost of a branch mattered, this could be alternative-patched
> >> out later on, but that seems silly.  I also bet that a more sensible
> >> fallback could be created in which printk would try to use an early
> >> console if there's no real console.
> >
> > So how about this:
> >
> > printk() does
> >
> >         vprintk_func = this_cpu_read(printk_func);
> >
> > and that's
> >
> > DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default
> >
> > I guess we can make that function be early_printk-something and once
> > printk is initialized, we overwrite it with vprintk_default.
> >
> > Elegant and no need for if branches and alternatives.
> >
> > Hmmm.
> 
> Jan, IIRC you were looking at printk recently-ish.  Any thoughts here?

Sounds like a good idea to me. I've also consulted this with Petr Mladek
(added to CC) who is using printk_func per-cpu variable in his
printk-from-NMI patches and he also doesn't see a problem with this.

I was just wondering about one thing - this way we add more early printks
if I understand your intention right. Are we guaranteed that they happen
only from a single CPU? Because currently there is no locking in
early_printk() and thus we can end up writing to early console several
messages in parallel from different CPUs. Not sure what's going to happen
in that case...

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR