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]

Message-ID: <YtrYdXWGb0NQLKNA@linutronix.de>
Date:   Fri, 22 Jul 2022 19:03:49 +0200
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Petr Mladek <pmladek@...e.com>
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 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. It sounds like we could not
> disable interrupts when interrupts or preemption is already disabled.
> Like nested disablement of interrupts is bad.
> 
> Is this a generic rule? Is is about the nesting?

You must not invoke the console drivers with disabled interrupts. This is
bad. So even if the context you were called from has interrupts enabled
then in console_emit_next_record() you have printk_safe_enter_irqsave()
before call_console_driver() which disables interrupts and this is bad.
More below…

> Or is is somehow specific to the console drivers called from printk()
> directly? Do you always want to disable here because it might
> be an atomic context and they might take too long?

You can't acquire a sleeping lock with disabled interrupts and or
preemption. Therefore the console drivers must not be invoked because
they need to acquire a sleeping lock(s).

> I guess that the sentence "additionally disables interrupts before
> it invokes the console drivers" is not really important" and it confused me.

This refers to printk_safe_enter_irqsave(). You could argue that it is
safe to invoke the console drivers if the context, in which printk() is
invoked, is safe. However this is not possible because printk disables
interrupts prio invoking the console drivers as just explained.
Therefore I don't see a way how to invoke the console drivers on RT as
of v5.19-rc7.

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

> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2843,6 +2843,16 @@ void console_unlock(void)
> >  	}
> >  
> >  	/*
> > +	 * On PREEMPT_RT it is not possible to invoke console drivers with
> > +	 * disabled interrupts and or preemption. Therefore all drivers are
> > +	 * skipped and the output can be retrieved from the buffer.
> > +	 */
> > +	if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
> > +		__console_unlock();
> > +		return;
> > +	}
> 
> Do you want this even in panic() or early boot?

yes. I don't see a way to invoke the console drivers without the
print-kthread.

> AFAIK, only the serial console has atomic write() callback in the RT
> tree. Is this the only console used by RT kernel users in practice?

The atomic console is made for emergencies. Everything else should be
written using the printk thread.

> 
> > +	/*
> >  	 * Console drivers are called with interrupts disabled, so
> >  	 * @console_may_schedule should be cleared before; however, we may
> >  	 * end up dumping a lot of lines, for example, if called from
> 
> Best Regards,
> Petr

Sebastian