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: <87leq1uev5.fsf@jogness.linutronix.de>
Date:   Fri, 30 Sep 2022 16:22:30 +0206
From:   John Ogness <john.ogness@...utronix.de>
To:     Petr Mladek <pmladek@...e.com>
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 2022-09-30, Petr Mladek <pmladek@...e.com> wrote:
> 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.

Agreed. I will do this for the next version.

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

No, let's not imagine this. It is déjà vu for the code that was
reverted.

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

We added mutex because list updates are always in may_sleep context and
we were moving to SRCU for list iteration. I think with v2, where SRCU
will be introduced earlier, things will be much clearer.

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

No, locking the list should have nothing to do with console_lock(). We
want to remove the list synchronization responsibilities from
console_lock(). In this series, I did not make that clear in the commit
messages. (Perhaps it was not entirely clear to me then.) For v2 I will
make this point very clear.

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

Sure. Removing console_lock() will be a long battle involving many
drivers. I am not trying to fight that battle right now. I just want
console_lock() out of the way of NOBKL consoles.

John