linux-kernel - Re: [PATCH] printk: drop in_nmi check from printk_safe_flush_on

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: <20180530084801.i5mzqhlw2w5o2eod@pathway.suse.cz>
Date:   Wed, 30 May 2018 10:48:01 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH] printk: drop in_nmi check from
 printk_safe_flush_on_panic()

On Wed 2018-05-30 16:51:05, Sergey Senozhatsky wrote:
> On (05/30/18 09:24), Petr Mladek wrote:
> > Acked-by: Petr Mladek <pmladek@...e.com>
> 
> Thanks.
> 
> > Just to be sure. IMHO, it is not worth nominating this patch for
> > stable. It is not a regression fix. I see it as a continuous
> > improving of the handling in various corner cases. And I see this
> > as a distant corner case.
> 
> Yep, agreed.
> 
> ***
> 
> A random thought [not suggesting anything]:
> 
> Given that we call printk() before SMP stop and that some of
> smp_send_stop() call printk(), may be we can switch panic()
> to printk_safe() mode and return it back to normal printk()
> mode right before printk_safe_flush_on_panic(). So all possible
> printk()-s that can happen in between (printk_safe_enter()
> printk_safe_exit()) will not access the logbuf spin lock, yet
> we still will try to flush all per-CPU buffers a bit later.
> 
> It probably doesn't sound like a very good/solid idea, just
> wondering what will people say.
> 
> Very schematically,
> 
> ---
> 
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 42e487488554..98a0493a59d3 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -148,6 +148,7 @@ void panic(const char *fmt, ...)
>  	 * after setting panic_cpu) from invoking panic() again.
>  	 */
>  	local_irq_disable();
> +	__printk_safe_enter();

I understand why you came with it but I am against this change without
a proper research. This would redirect too valuable messages into
a buffer of a limited size and postpone flushing them to the consoles.

We would need to really carefully compare chances where this would
help and where it would make things worse. There is a high chance
that we could come with a better solution once we have the analyze.

Best Regards,
Petr