linux-kernel - [PATCH printk v3 07/40] console: introduce console_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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon,  7 Nov 2022 15:22:05 +0106
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: [PATCH printk v3 07/40] console: introduce console_is_enabled() wrapper

After switching to SRCU for console list iteration, some readers
will begin readings console->flags as a data race. Locklessly
reading console->flags provides a consistent value because there
is at most one CPU modifying console->flags and that CPU is
using only read-modify-write operations.

The primary reason for readers to access console->flags is to
check if the console is enabled. Introduce console_is_enabled()
to mark such access as a data race.

Signed-off-by: John Ogness <john.ogness@...utronix.de>
---
 include/linux/console.h | 27 +++++++++++++++++++++++++++
 kernel/printk/printk.c  | 10 +++++-----
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/include/linux/console.h b/include/linux/console.h
index f4f0c9523835..d9c636011364 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -172,6 +172,33 @@ extern void console_srcu_read_unlock(int cookie);
 
 extern struct hlist_head console_list;
 
+/**
+ * console_is_enabled - Locklessly check if the console is enabled
+ * @con:	struct console pointer of console to check
+ *
+ * Unless the caller is explicitly synchronizing against the console
+ * register/unregister/stop/start functions, this function should be
+ * used instead of manually readings console->flags and testing for
+ * the CON_ENABLED bit.
+ *
+ * This function provides the necessary READ_ONCE() and data_race()
+ * notation for locklessly reading the console flags. The READ_ONCE()
+ * in this function matches the WRITE_ONCE() when @flags are modified
+ * for registered consoles.
+ *
+ * Context: Any context.
+ */
+static inline bool console_is_enabled(const struct console *con)
+{
+	/*
+	 * Locklessly reading console->flags provides a consistent
+	 * read value because there is at most one CPU modifying
+	 * console->flags and that CPU is using only read-modify-write
+	 * operations to do so.
+	 */
+	return (data_race(READ_ONCE(con->flags)) & CON_ENABLED);
+}
+
 /**
  * for_each_console_srcu() - Iterator over registered consoles
  * @con:	struct console pointer used as loop cursor
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8974523f3107..79811984da34 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3021,7 +3021,7 @@ void console_stop(struct console *console)
 {
 	__pr_flush(console, 1000, true);
 	console_lock();
-	console->flags &= ~CON_ENABLED;
+	WRITE_ONCE(console->flags, console->flags & ~CON_ENABLED);
 	console_unlock();
 
 	/*
@@ -3037,7 +3037,7 @@ EXPORT_SYMBOL(console_stop);
 void console_start(struct console *console)
 {
 	console_lock();
-	console->flags |= CON_ENABLED;
+	WRITE_ONCE(console->flags, console->flags | CON_ENABLED);
 	console_unlock();
 	__pr_flush(console, 1000, true);
 }
@@ -3256,7 +3256,7 @@ void register_console(struct console *newcon)
 
 	} else if (newcon->flags & CON_CONSDEV) {
 		/* Only the new head can have CON_CONSDEV set. */
-		console_first()->flags &= ~CON_CONSDEV;
+		WRITE_ONCE(console_first()->flags, console_first()->flags & ~CON_CONSDEV);
 		hlist_add_head_rcu(&newcon->node, &console_list);
 
 	} else {
@@ -3308,7 +3308,7 @@ int unregister_console(struct console *console)
 	console_lock();
 
 	/* Disable it unconditionally */
-	console->flags &= ~CON_ENABLED;
+	WRITE_ONCE(console->flags, console->flags & ~CON_ENABLED);
 
 	if (hlist_unhashed(&console->node)) {
 		console_unlock();
@@ -3327,7 +3327,7 @@ int unregister_console(struct console *console)
 	 * console has any device attached. Oh well....
 	 */
 	if (!hlist_empty(&console_list) && console->flags & CON_CONSDEV)
-		console_first()->flags |= CON_CONSDEV;
+		WRITE_ONCE(console_first()->flags, console_first()->flags | CON_CONSDEV);
 
 	console_unlock();
 
-- 
2.30.2