linux-kernel - Re: [PATCH -printk] printk, tracing: fix console tracepoint

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: <20220713112541.GB2737@pathway.suse.cz>
Date:   Wed, 13 Jul 2022 13:25:41 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Marco Elver <elver@...gle.com>,
        John Ogness <john.ogness@...utronix.de>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        linux-kernel@...r.kernel.org, kasan-dev@...glegroups.com,
        Thomas Gleixner <tglx@...utronix.de>,
        Johannes Berg <johannes.berg@...el.com>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Naresh Kamboju <naresh.kamboju@...aro.org>,
        Linux Kernel Functional Testing <lkft@...aro.org>
Subject: Re: [PATCH -printk] printk, tracing: fix console tracepoint

On Tue 2022-07-12 08:16:55, Paul E. McKenney wrote:
> On Tue, Jul 12, 2022 at 10:53:53AM -0400, Steven Rostedt wrote:
> > On Tue, 12 Jul 2022 06:49:16 -0700
> > "Paul E. McKenney" <paulmck@...nel.org> wrote:
> > 
> > > > I guess the question is, can we have printk() in such a place? Because this
> > > > tracepoint is attached to printk and where ever printk is done so is this
> > > > tracepoint.  
> > > 
> > > As I understand it, code in such a place should be labeled noinstr.
> > > Then the call to printk() would be complained about as an illegal
> > > noinstr-to-non-noinstr call.
> > > 
> > > But where exactly is that printk()?
> > 
> > Perhaps the fix is to remove the _rcuidle() from trace_console_rcuidle().
> > If printk() can never be called from noinstr (aka RCU not watching).
> 
> Maybe printk() is supposed to be invoked from noinstr.  It might be a
> special case in the tooling.  I have no idea.  ;-)

I think that it is ok to do _not_ support printk() in noinstr parts.

> However, the current SRCU read-side algorithm will tolerate being invoked
> from noinstr as long as it is not also an NMI handler.  Much though
> debugging tools might (or might not) complain.
> 
> Don't get me wrong, I can make SRCU tolerate being called while RCU is
> not watching.  It is not even all that complicated.  The cost is that
> architectures that have NMIs but do not have NMI-safe this_cpu*()
> operations have an SRCU reader switch from explicit smp_mb() and
> interrupt disabling to a cmpxchg() loop relying on the implicit barriers
> in cmpxchg().
> 
> For arm64, this was reportedly a win.

IMHO, the tracepoint in printk() is not worth slowing down other
important fast paths.

The tracepoint was moved into vprintk_store() in 5.19-rc1. It used
to be in console_unlock() before. The previous location was not
reliable by definition. Old messages might be overridden by new
ones before they reach console. Also messages in NMI context
used to be stored in per-CPU buffers. There was even bigger
risk that they would not reach the console.

Best Regards,
Petr