linux-kernel - Re: [Question] printk_safe: Do you want synch./barriers in raw_spin_is

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>] [day] [month] [year] [list]

Date:   Wed, 7 Mar 2018 11:31:23 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Andrea Parri <parri.andrea@...il.com>,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [Question] printk_safe: Do you want synch./barriers in
 raw_spin_is_locked()

Hello,

I'll Cc linux-kernel

On (03/06/18 17:21), Steven Rostedt wrote:
> On Tue, 6 Mar 2018 15:58:46 +0100
> Andrea Parri <parri.andrea@...il.com> wrote:
> 
> > Dear PRINTK maintainers,
> > 
> > Following a recent discussion on LKML[1], I started auditing callsites
> > of spin_is_locked() and the "implicit" assumptions these sites made on
> > the memory ordering enforced by this primitive (not a first attempt[2],
> > FWIW).  As it turns out, this primitive is mostly used (40+ calls) for
> > debugging purposes (WARN_ON(!spin_is_locked()) or such), but the calls
> > to raw_spin_is_locked() in printk_safe seem to escape this usage.
> > 
> > Which assumptions are you relying on (if any) for raw_spin_is_locked()
> > enforced memory ordering?
> 
> We don't care about other CPUs. The use case here is to make sure that
> the printk in an NMI does not take the lock when the current CPU has
> it. Memory ordering is fine when dealing with only one CPU. If other
> CPUs mess with the result, then the worse that will happen is that we
> go to the "safe" mode when we didn't have to.

Right, thanks Steven.

A side note,
I think the only reason we have that raw_spin_is_locked() is because we
call console drivers in printk-safe mode, so we don't have a 1:1 mapping:

		printk-safe == raw_spin_is_locked(logbuf)

I sort of think we can stop calling console drivers in printk-safe,
there seems to be no real reasons to do so (kind of). And then we will
have that missing printk-safe == raw_spin_is_locked(logbuf) so we will
be able to drop raw_spin_is_locked() from printk-safe and make logbuf
spin_lock static again.

> BTW, next time when asking a question, don't do it off list. This may
> be something others would like to know. I usually don't answer
> questions like this when they don't include a Cc to a mailing list.

Agreed.

	-ss