linux-kernel - atomic_flush vs boot consoles - was: Re: [PATCH printk v2 05/18] printk: Atomic print in printk context on shutdown

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: Thu, 13 Jun 2024 14:44:53 +0200
From: Petr Mladek <pmladek@...e.com>
To: John Ogness <john.ogness@...utronix.de>
Cc: Sergey Senozhatsky <senozhatsky@...omium.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org
Subject: atomic_flush vs boot consoles - was: Re: [PATCH printk v2 05/18]
 printk: Atomic print in printk context on shutdown

On Tue 2024-06-04 01:30:40, John Ogness wrote:
> For nbcon consoles, normally the printing is handled by the
> dedicated console printing threads. However, on shutdown the
> printing threads may not get a chance to print the final
> messages.
> 
> When shutting down or rebooting (system_state > SYSTEM_RUNNING),
> perform atomic printing from the printk() caller context.
> 
> Signed-off-by: John Ogness <john.ogness@...utronix.de>
> ---
>  kernel/printk/nbcon.c  | 5 +++--
>  kernel/printk/printk.c | 7 ++++++-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2389,12 +2389,17 @@ asmlinkage int vprintk_emit(int facility, int level,
>  		 *
>  		 * - When this CPU is in panic.
>  		 *
> +		 * - During shutdown, since the printing threads may not get
> +		 *   a chance to print the final messages.
> +		 *
>  		 * Note that if boot consoles are registered, the console
>  		 * lock/unlock dance must be relied upon instead because nbcon
>  		 * consoles cannot print simultaneously with boot consoles.
>  		 */
> -		if (is_panic_context)
> +		if (is_panic_context ||
> +		    (system_state > SYSTEM_RUNNING)) {
>  			nbcon_atomic_flush_pending();
> +		}
>  	}

I was curious why we do not need to add this check also to
console_flush_all() which checks con->kthread as well.

I realized that we call it only when

  #define printing_via_unlock (have_legacy_console || have_boot_console)

=> it handles nbcon consoles only when there is still a boot console.

It actually have to handle them in this case because the boot consoles
can be serialized only via console_lock().

In fact, nbcon_atomic_flush_pending() is not safe when there are boot
consoles. This is handled in vprintk_emit(). But it is not enforced
on several other locations.

I played with a possible solutions. The conditions around the console
calls are getting more and more complicated. I am going to send a
solution which looks acceptable to me.

Best Regards,
Petr