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]

Date: Fri, 28 Jun 2024 14:28:47 +0206
From: John Ogness <john.ogness@...utronix.de>
To: Petr Mladek <pmladek@...e.com>
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 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().

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.

John