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 16:17:13 +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:
> 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?

No.

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

On PREEMPT_RT, legacy consoles are not allowed to print from
non-preemptible contexts because they use spinlocks (rtmutexes).

This is a disadvantage for legacy consoles on PREEMPT_RT. If people want
a legacy console to gain the reliability attribute on PREEMPT_RT, then
that console _must_ be converted to nbcon.

force_printkthreads() is used to establish the same console printing
behavior as PREEMPT_RT. (A later patch makes this behavior available to
all preemption models so that users can enjoy the non-interference
attribute of the rework.)

>> 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?

In the threaded model, the thread is a critical component of the
console. If the thread cannot start, it is the same as if memory could
not be allocated. The registration must fail. And just as it is now, the
user will only see that it failed via dmesg (or some other console that
did not fail).

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

That is not the issue. The problem is that you want legacy consoles in
RT to print without the printing thread. That is not possible. Those
drivers are not RT compatible and will cause deadlocks. They _must_ be
modified i.e. converted to nbcon consoles.

We have worked really hard to allow legacy consoles to work "as is" for
!PREEMPT_RT. But when PREEMPT_RT is enabled legacy consoles must be
forced into the thread (except for panic, where we close our eyes and
hope for the best).

John