linux-kernel - Re: [PATCH printk v2 12/18] printk: Add kthread for all legacy consoles

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: <Zn67hDCEHdgtYPv3@pathway.suse.cz>
Date: Fri, 28 Jun 2024 15:32:52 +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 v2 12/18] printk: Add kthread for all legacy
 consoles

On Fri 2024-06-28 14:28:47, John Ogness wrote:
> On 2024-06-28, Petr Mladek <pmladek@...e.com> wrote:
> >> @@ -1494,7 +1508,9 @@ void nbcon_cpu_emergency_exit(void)
> >>  		 * to handle it.
> >>  		 */
> >>  		do_trigger_flush = true;
> >> -		if (printing_via_unlock && !is_printk_deferred()) {
> >> +		if (!force_printkthreads() &&
> >
> > Is this correct? We still need to flush the messages directly
> > when the legacy kthread is not ready yet.
> 
> No! If force_printkthreads() is set, messages will never flush directly
> except for console_flush_on_panic().

But this is an _emergency_ context! This would be inconsistent with
the nbcon consoles where the messages are flushed directly.

Is RT sheduling quaranteed when the system reached emergency state?

In fact, we probably should not check force_printkthreads() here at
all. It would be only for NBCON_PRIO_NORMAL context.

Or the option should force the kthreads even for nbcon consoles.


> I see that I implemented get_init_console_seq() to flush since it is
> known to be in task state. But that is wrong. Also there it should not
> flush directly. It is not about whether or not printing is safe. It is
> about deferring to the thread 100% of the time except for panic.
> 
> >> +void nbcon_legacy_kthread_create(void)
> >> +{
> >> +	struct task_struct *kt;
> >> +
> >> +	lockdep_assert_held(&console_mutex);
> >> +
> >> +	if (!force_printkthreads())
> >> +		return;
> >> +
> >> +	if (!printk_threads_enabled || nbcon_legacy_kthread)
> >> +		return;
> >> +
> >> +	kt = kthread_run(nbcon_legacy_kthread_func, NULL, "pr/legacy");
> >> +	if (IS_ERR(kt)) {
> >> +		pr_err("unable to start legacy printing thread\n");
> >> +		return;
> >
> > Is this acceptable for RT, please?
> 
> It is not acceptable for mainline. For the next version, any failed
> thread creation leads to unregistering the console. In the case of the
> legacy thread, it will mean unregistering all legacy consoles on
> failure.

It means that the system would become silent. Is this a good idea?

IMHO, we have a fundamental problem here.
Are RT guarantees more important than printk() or vice versa?

What is happening when the RT guarantees are violated?
For example, when the scheduler detects a problem?
Does it panic() or tries to continue?

>From my POV, silent system is the worst solution. The user
might think: "no messages, no problem".

Best Regards,
Petr