linux-kernel - Re: [PATCH] printk: nbcon_atomic_flush_pending() is safe only when there is no boot console

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: Tue, 25 Jun 2024 22:59:34 +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
Subject: Re: [PATCH] printk: nbcon_atomic_flush_pending() is safe only when
 there is no boot console

On 2024-06-13, Petr Mladek <pmladek@...e.com> wrote:
> Boot consoles are not serialized with the nbcon consoles via the nbcon
> console context or con->device_lock(). The serialization is possible only
> via the legacy console_lock().
>
> The decision whether nbcon_atomic_flush_pending() should and can be
> called safely is similar and closely related to the decision
> whether the legacy loop should be used.
>
> Define printing_via_context_safe symmetrically with printing_via_unlock.
> Allow to call nbcon_atomic_flush_pending() only when it is needed and safe.

This patch, along with other comments you made about the many different
checks to see when it is allowed to do what, forced me to take a step
back and look at the big picture. Indeed, by the end of this series we
have many boolean variables that influence decisions about how to handle
threads and how to print.

I am thinking it makes more sense to incorporate these booleans into a
single variable (printk_state?). The variable would only be changed
under the console_list_lock, which could allow lockless users to
leverage the console_srcu_read_lock for consistency.

The different orthogonal bits of the variable would be:

* have_boot_console - true if a boot console is registered

* have_legacy_console - true if a non-nbcon console is registered

* have_nbcon_console - true if an nbcon console is registered

* printk_threads_allowed - true when it is OK to have threads
  (set/cleared by notifiers, and more or less represents
  "system_state == SYSTEM_SCHEDULING || system_state == SYSTEM_RUNNING")

* printk_threads_available - true while all printing threads are running
  (and implies at least 1 thread is running)

We could provide macros to interpret this multi-flag value for various
scenarios:

#define nbcon_may_create_threads()
  (printk_threads_allowed)

#define nbcon_may_rely_on_threads()
  (have_nbcon_console && !have_boot_console && printk_threads_available)

#define nbcon_may_flush_atomic()
  (have_nbcon_console && !have_boot_console)

#define nbcon_must_flush_atomic()
  (have_nbcon_console && !have_boot_console && !printk_threads_available)

#define nbcon_must_flush_via_unlock()
  (have_nbcon_console && have_boot_console)

#define printing_via_unlock()
  (have_legacy_console || have_boot_console)

Of course, the exact naming of these macros gives us a big shed to
paint. But I think this could help to simplify/unify the checks for the
various scenarios.

John Ogness