linux-kernel - Re: [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context

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] [day] [month] [year] [list]

Message-ID: <aNp1YogXZh6dObzI@pathway.suse.cz>
Date: Mon, 29 Sep 2025 14:02:42 +0200
From: Petr Mladek <pmladek@...e.com>
To: John Ogness <john.ogness@...utronix.de>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Esben Haabendal <esben@...nix.com>, linux-serial@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Arnd Bergmann <arnd@...db.de>, Tony Lindgren <tony@...mide.com>,
	Niklas Schnelle <schnelle@...ux.ibm.com>,
	Serge Semin <fancer.lancer@...il.com>,
	Andrew Murray <amurray@...goodpenguin.co.uk>
Subject: Re: [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is
 in an emergency context

On Fri 2025-09-26 16:43:33, John Ogness wrote:
> On 2025-09-26, Petr Mladek <pmladek@...e.com> wrote:
> > In emergency contexts, printk() tries to flush messages directly even
> > on nbcon consoles. And it is allowed to takeover the console ownership
> > and interrupt the printk kthread in the middle of a message.
> >
> > Only one takeover and one repeated message should be enough in most
> > situations. The first emergency message flushes the backlog and printk
> > kthreads get to sleep. Next emergency messages are flushed directly
> > and printk() does not wake up the kthreads.
> >
> > However, the one takeover is not guaranteed. Any printk() in normal
> > context on another CPU could wake up the kthreads. Or a new emergency
> > message might be added before the kthreads get to sleep. Note that
> > the interrupted .write_kthread() callbacks usually have to call
> 
>                   .write_thread()

Oh my muscle memory ;-)

> > nbcon_reacquire_nobuf() and restore the original device setting
> > before checking for pending messages.

[...]

> > --- a/kernel/printk/nbcon.c
> > +++ b/kernel/printk/nbcon.c
> > @@ -1674,10 +1696,18 @@ void nbcon_cpu_emergency_exit(void)
> >  	unsigned int *cpu_emergency_nesting;
> >  
> >  	cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
> > -
> >  	if (!WARN_ON_ONCE(*cpu_emergency_nesting == 0))
> >  		(*cpu_emergency_nesting)--;
> >  
> > +	/*
> > +	 * Wake up kthreads because there might be some pending messages
> > +	 * added by other CPUs with normal priority since the last flush
> > +	 * in the emergency context.
> > +	 */
> > +	if (!WARN_ON_ONCE(atomic_read(&nbcon_cpu_emergency_cnt) == 0))
> > +		if (atomic_dec_return(&nbcon_cpu_emergency_cnt) == 0)
> > +			nbcon_kthreads_wake();
> 
> Although technically it doesn't hurt to blindly call
> nbcon_kthreads_wake(), you may want to do it more formally. Maybe like
> this:
> 
> 	if (!WARN_ON_ONCE(atomic_read(&nbcon_cpu_emergency_cnt) == 0)) {
> 		if (atomic_dec_return(&nbcon_cpu_emergency_cnt) == 0) {
> 			struct console_flush_type ft;
> 
> 			printk_get_console_flush_type(&ft);
> 			if (ft.nbcon_offload)
> 				nbcon_kthreads_wake();
> 		}
> 	}
> 
> I leave it up to you.

I agree that this is better. I'll use it in v2.

> With the static+initializer change:
> 
> Reviewed-by: John Ogness <john.ogness@...utronix.de>

Thanks a lot for quick review.

I am going to send v2 when the panic state API patchset (in -mm tree)
gets accepted upstream.

Best Regards,
Petr