linux-kernel - Re: [PATCH v2] printk: Skip console drivers on PREEMPT

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:   Mon, 25 Jul 2022 14:30:04 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     John Ogness <john.ogness@...utronix.de>,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v2] printk: Skip console drivers on PREEMPT_RT.

On Fri 2022-07-22 19:03:49, Sebastian Andrzej Siewior wrote:
> On 2022-07-22 14:39:44 [+0200], Petr Mladek wrote:
> > On Thu 2022-07-21 08:50:38, Sebastian Andrzej Siewior wrote:
> > > printk might be invoked in a context with disabled interrupts and or
> > > preemption and additionally disables interrupts before it invokes the
> > > console drivers. This is behaviour is not compatible with PREEMPT_RT.
> > 
> > Maybe I do not understand it correctly.
> 
> > Or is is somehow specific to the console drivers called from printk()
> > directly?
> 
> You can't acquire a sleeping lock with disabled interrupts and or

This is what I have missed. It might be obvious for people living by RT
kernel. But spinlocks do not sleep in normal kernel. So I did not get
that the spinlocks are the culprit. Please, make it more obvious
in the commit message, for example:

--- cut ---
Console drivers use spinlocks that might sleep in PREEMPT_RT. As a
result they must not be called with interrupts enabled...
--- cut ---

> > > Disable console printing until the return of atomic consoles and the
> > > printing thread. This allows to retrieve the log buffer from user space
> > > which is not possible by disable printk.
> > 
> > I guess that this is for RT tree because the kthreads and the atomic
> > consoles are still not in the mainline.
> 
> I would like to have this applied to the v5.20 upstream tree and then
> revoked once the missing bits have been  merged. Based on what I see,
> there shouldn't be any road blocks.

Huh, I do not think that it is a good idea. There are neither atomic
consoles nor printk kthreads in upstream. The patch effectively
completely disables the consoles in PREEMPT_RT. All consoles
will be _empty all the time_.

Also the consoles were never tested with interrupts enabled. I am
pretty sure that interrupts has to be disabled in some locations,
for example, when sending some data sequences on the ports. Are we
sure that they are always explicitly disabled inside the drivers?
This is one reason why we reverted the console kthreads in 5.19-rc4.
AFAIK, John is doing some inspection of all drivers.

That said, I am going to leave the decision on John. I am not sure
what are the expectations of RT users in mainline.

>From my point, this patch does not make much sense. IMHO, it will
not make mainline usable with PREEMPT_RT. Any serious RT user will
need to revert it and apply a better printk solution from
the out-of-tree RT patchset.

Best Regards,
Petr