linux-kernel - Re: [PATCH printk 06/18] printk: Protect [un]register

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: <Yza1wxVcH2bsITcs@alley>
Date:   Fri, 30 Sep 2022 11:24:19 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH printk 06/18] printk: Protect [un]register_console() with
 a mutex

On Thu 2022-09-29 17:43:17, Petr Mladek wrote:
> On Thu 2022-09-29 01:48:29, John Ogness wrote:
> > On 2022-09-27, Petr Mladek <pmladek@...e.com> wrote:
> > > Hmm, the new mutex is really nasty. It has very strange semantic.
> > > It makes the locking even more complicated.
> > 
> > We are working to replace the BKL-console_lock with new separate clearly
> > defined mechanisms.
> >
> > The new mutex provides full synchronization for list changes as well as
> > changes to items of that list. (Really console->flags is the only change
> > to items of the list.)

We should actually make the the reading of console->flags safe under
srcu_read_lock(). It would allow to use the SRCU walk by all the
readers.

> > For some places in the code it is very clear that the console_lock can
> > be completely replaced (either with srcu or the new mutex). For other
> > places, it is not yet clear why the console_lock is being used and so
> > both console_lock and mutex are used.
> 
> One important and tricky location is console_trylock() in
> vprintk_emit(). And the related for_each_console() called from
> console_unlock()->console_flush_all().
> 
> It is the legacy mode that tries to print to the consoles immediately.
> I am not sure if we could _ever_ remove this mode.
> 
> I would really like to avoid state where we have two locks (semaphore
> and mutex) serializing the same thing (console list).

That said, I could imagine implementing console_lock() so that it
would be implemented by mutex when the legacy mode is disabled and
semaphore when it is allowed.

You were talking about command-line option that would allow to
disable the legacy mode on production RT systems. And I guess
that you added mutex because it behaves better on RT.

Also I could imagine using console_list_lock() as a wrapper
to console_lock(). It might help to distinguish locations where
the list is traversed and where the console_lock() is used for
another reason. I mean to remove the big-kernel-lock character
of the console_lock().

You know, the more locks we have, the bigger is the risk of
deadlocks, and the more hacks would be needed in
console_flush_on_panic(). And I am afraid
that console_lock() will be with us for many years and
maybe forever.

Does it make any sense, please?

Best Regards,
Petr