linux-kernel - Re: [PATCH printk v4 09/17] printk: nbcon: Rely on kthreads for normal operation

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: <Zs31xM0Nn0hEb74O@pathway.suse.cz>
Date: Tue, 27 Aug 2024 17:50:28 +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: Re: [PATCH printk v4 09/17] printk: nbcon: Rely on kthreads for
 normal operation

On Tue 2024-08-27 06:49:25, John Ogness wrote:
> Once the kthread is running and available
> (i.e. @printk_kthreads_running is set), the kthread becomes
> responsible for flushing any pending messages which are added
> in NBCON_PRIO_NORMAL context. Namely the legacy
> console_flush_all() and device_release() no longer flush the
> console. And nbcon_atomic_flush_pending() used by
> nbcon_cpu_emergency_exit() no longer flushes messages added
> after the emergency messages.
> 
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2749,6 +2753,10 @@ void resume_console(void)
>  	 */
>  	synchronize_srcu(&console_srcu);
>  
> +	printk_get_console_flush_type(&ft);
> +	if (ft.nbcon_offload)
> +		nbcon_kthreads_wake();

Who would flush the nbcon consoles in the following case?

  ft.nbcon_atomic == true && ft.legacy_direct == false

pr_flush() won't call the legacy loop in this case.

We should probably update pr_flush() to call nbcon_atomic_flush_pending()
in this case.

> +
>  	pr_flush(1000, true);
>  }
>  
> @@ -3387,9 +3405,25 @@ EXPORT_SYMBOL(console_stop);
>  
>  void console_start(struct console *console)
>  {
> +	struct console_flush_type ft;
> +	bool is_nbcon;
> +
>  	console_list_lock();
>  	console_srcu_write_flags(console, console->flags | CON_ENABLED);
> +	is_nbcon = console->flags & CON_NBCON;
>  	console_list_unlock();
> +
> +	/*
> +	 * Ensure that all SRCU list walks have completed. The related
> +	 * printing context must be able to see it is enabled so that
> +	 * it is guaranteed to wake up and resume printing.
> +	 */
> +	synchronize_srcu(&console_srcu);
> +
> +	printk_get_console_flush_type(&ft);
> +	if (is_nbcon && ft.nbcon_offload)
> +		nbcon_kthread_wake(console);

Same here.

>  	__pr_flush(console, 1000, true);
>  }
>  EXPORT_SYMBOL(console_start);
> @@ -4629,8 +4665,13 @@ EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
>   */
>  void console_try_replay_all(void)
>  {
> +	struct console_flush_type ft;
> +
> +	printk_get_console_flush_type(&ft);
>  	if (console_trylock()) {
>  		__console_rewind_all();
> +		if (ft.nbcon_offload)
> +			nbcon_kthreads_wake();

And here. We in this case, we should likely call add here:

		if (ft.nbcon_atomic)
			nbcon_atomic_flush_pending()
			
>  		/* Consoles are flushed as part of console_unlock(). */
>  		console_unlock();
>  	}

Best Regards,
Petr