linux-kernel - Re: [RFC PATCH v1 20/25] serial: 8250: implement write

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, 8 Mar 2019 11:28:19 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Daniel Wang <wonderfly@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Cox <gnomes@...rguk.ukuu.org.uk>,
        Jiri Slaby <jslaby@...e.com>,
        Peter Feiner <pfeiner@...gle.com>,
        linux-serial@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [RFC PATCH v1 20/25] serial: 8250: implement write_atomic

On Fri 2019-03-08 05:05:12, John Ogness wrote:
> On 2019-02-27, Petr Mladek <pmladek@...e.com> wrote:
> >>>> Implement a non-sleeping NMI-safe write_atomic console function in
> >>>> order to support emergency printk messages.
> >>>
> >>> OK, it would be safe when prb_lock() is the only lock taken
> >>> in the NMI handler.
> >> 
> >> Which is the case. As I wrote to you already [0], NMI contexts are
> >> _never_ allowed to do things that rely on waiting forever for other
> >> CPUs.
> >
> > Who says _never_? I agree that it is not reasonable. But the
> > history shows that it happens.
> 
> Right, which is why it would need to become policy.
> 
> The emergency messages (aka write_atomic) introduce a new requirement to
> the kernel because this callback must be callable from any context. The
> console drivers must have some way of synchronizing. The CPU-reentrant
> spin lock is the only solution I am aware of.

I am not in position to decide if this requirement is acceptable
or not. We would need an opinion from Linus at minimum.

But it is not acceptable from my point of view. Note that
many spinlocks might be safely used in NMI in principle.

You want to introduce a single spinlock just because
printk() could be called anywhere. Most other subsystems
do not have this problem because they a more self-contained.

> If the ringbuffer was fully lockless

Exactly, I think that a solution would be fully lockless logbuffer.


> we should be able to have per-console CPU-reentrant spin locks
> as long as the ordering is preserved, which I expect shouldn't
> be a problem. If any NMI context needed a spin lock for its own
> purposes, it would need to use the CPU-reentrant spin lock of
> the first console so as to preserve the ordering in case of a panic.

IMHO, this is not acceptable. It would be nice to have direct output
from NMI but the cost looks to high here.

A solution would be to defer the console to irq_work or so.
We might also consider using trylock in NMI and have
the irq_work just as a fallback.

Best Regars,
Petr