[PATCH printk v2 33/38] printk: introduce console_list_lock

John Ogness
2022-10-19T15:07:24Z

Currently there exist races in console_register(), where the types
of registered consoles are checked (without holding the console_lock)
and then after acquiring the console_lock, it is assumed that the list
has not changed. Also, some code that performs console_unregister()
make similar assumptions.

Introduce a console_list_lock to provide full synchronization for any
console list changes. The console_list_lock also provides
synchronization for updates to console->flags.

Note that one of the various responsibilities of the console_lock is
also intended to provide this synchronization. Later changes will
update call sites relying on the console_lock for this purpose. Once
all call sites have been updated, the console_lock will be relieved
of synchronizing console_list and console->flags updates.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
include/linux/console.h | 20 ++++++++--
kernel/printk/printk.c | 82 +++++++++++++++++++++++++++++++++++------
2 files changed, 88 insertions(+), 14 deletions(-)

diff --git a/include/linux/console.h b/include/linux/console.h
index 60195cd086dc..bf1e8136424a 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -159,8 +159,12 @@ struct console {
};

#ifdef CONFIG_LOCKDEP
+extern void lockdep_assert_console_list_lock_held(void);
extern bool console_srcu_read_lock_is_held(void);
#else
+static inline void lockdep_assert_console_list_lock_held(void)
+{
+}
static inline bool console_srcu_read_lock_is_held(void)
{
return 1;
@@ -170,6 +174,9 @@ static inline bool console_srcu_read_lock_is_held(void)
extern int console_srcu_read_lock(void);
extern void console_srcu_read_unlock(int cookie);

+extern void console_list_lock(void) __acquires(console_mutex);
+extern void console_list_unlock(void) __releases(console_mutex);
+
extern struct hlist_head console_list;

/**
@@ -206,10 +213,17 @@ static inline bool console_is_enabled(const struct console *con)
hlist_for_each_entry_srcu(con, &console_list, node, \
console_srcu_read_lock_is_held())

-/*
- * for_each_console() allows you to iterate on each console
+/**
+ * for_each_console() - Iterator over registered consoles
+ * @con: struct console pointer used as loop cursor
+ *
+ * The console list and all struct console fields are immutable while
+ * iterating.
+ *
+ * Requires console_list_lock to be held.
*/
-#define for_each_console(con) \
+#define for_each_console(con) \
+ lockdep_assert_console_list_lock_held(); \
hlist_for_each_entry(con, &console_list, node)

extern int console_set_on_cmdline;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2faa6e3e2fb8..3615ee6d68fd 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -78,6 +78,9 @@ EXPORT_SYMBOL(ignore_console_lock_warning);
int oops_in_progress;
EXPORT_SYMBOL(oops_in_progress);

+/* console_mutex protects console_list and console->flags updates. */
+static DEFINE_MUTEX(console_mutex);
+
/*
* console_sem protects console_list and console->flags updates, and also
* provides serialization for access to the entire console driver system.
@@ -104,6 +107,11 @@ static struct lockdep_map console_lock_dep_map = {
.name = "console_lock"
};

+void lockdep_assert_console_list_lock_held(void)
+{
+ lockdep_assert_held(&console_mutex);
+}
+
bool console_srcu_read_lock_is_held(void)
{
return srcu_read_lock_held(&console_srcu);
@@ -225,6 +233,40 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
}
#endif /* CONFIG_PRINTK && CONFIG_SYSCTL */

+/**
+ * console_list_lock - Lock the console list
+ *
+ * For console list or console->flags updates
+ */
+void console_list_lock(void)
+{
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ /*
+ * In unregister_console(), synchronize_srcu() is called with the
+ * console_list_lock held. Therefore it is not allowed that the
+ * console_list_lock is taken with the srcu_lock held.
+ *
+ * Whether or not this context is in the read-side critical section
+ * can only be detected if the appropriate debug options are enabled.
+ */
+ WARN_ON_ONCE(debug_lockdep_rcu_enabled() &&
+ srcu_read_lock_held(&console_srcu));
+#endif
+ mutex_lock(&console_mutex);
+}
+EXPORT_SYMBOL(console_list_lock);
+
+/**
+ * console_list_unlock - Unlock the console list
+ *
+ * Counterpart to console_list_lock()
+ */
+void console_list_unlock(void)
+{
+ mutex_unlock(&console_mutex);
+}
+EXPORT_SYMBOL(console_list_unlock);
+
/**
* console_srcu_read_lock - Register a new reader for the
* SRCU-protected console list
@@ -3055,9 +3097,11 @@ struct tty_driver *console_device(int *index)
void console_stop(struct console *console)
{
__pr_flush(console, 1000, true);
+ console_list_lock();
console_lock();
console->flags &= ~CON_ENABLED;
console_unlock();
+ console_list_unlock();

/* Ensure that all SRCU list walks have completed */
synchronize_srcu(&console_srcu);
@@ -3066,9 +3110,11 @@ EXPORT_SYMBOL(console_stop);

void console_start(struct console *console)
{
+ console_list_lock();
console_lock();
console->flags |= CON_ENABLED;
console_unlock();
+ console_list_unlock();
__pr_flush(console, 1000, true);
}
EXPORT_SYMBOL(console_start);
@@ -3164,6 +3210,8 @@ static void try_enable_default_console(struct console *newcon)
#define console_first() \
hlist_entry(console_list.first, struct console, node)

+static int unregister_console_locked(struct console *console);
+
/*
* The console driver calls this routine during kernel initialization
* to register the console printing procedure with printk() and to
@@ -3188,15 +3236,14 @@ void register_console(struct console *newcon)
struct console *con;
bool bootcon_enabled = false;
bool realcon_enabled = false;
- int cookie;
int err;

- cookie = console_srcu_read_lock();
- for_each_console_srcu(con) {
+ console_list_lock();
+
+ for_each_console(con) {
if (WARN(con == newcon, "console '%s%d' already registered\n",
con->name, con->index)) {
- console_srcu_read_unlock(cookie);
- return;
+ goto unlock;
}

if (con->flags & CON_BOOT)
@@ -3204,13 +3251,12 @@ void register_console(struct console *newcon)
else
realcon_enabled = true;
}
- console_srcu_read_unlock(cookie);

/* Do not register boot consoles when there already is a real one. */
if (newcon->flags & CON_BOOT && realcon_enabled) {
pr_info("Too late to register bootconsole %s%d\n",
newcon->name, newcon->index);
- return;
+ goto unlock;
}

/*
@@ -3241,7 +3287,7 @@ void register_console(struct console *newcon)

/* printk() messages are not printed to the Braille console. */
if (err || newcon->flags & CON_BRL)
- return;
+ goto unlock;

/*
* If we have a bootconsole, and are switching to a real console,
@@ -3304,13 +3350,15 @@ void register_console(struct console *newcon)

hlist_for_each_entry_safe(con, tmp, &console_list, node) {
if (con->flags & CON_BOOT)
- unregister_console(con);
+ unregister_console_locked(con);
}
}
+unlock:
+ console_list_unlock();
}
EXPORT_SYMBOL(register_console);

-int unregister_console(struct console *console)
+static int unregister_console_locked(struct console *console)
{
int res;

@@ -3362,6 +3410,16 @@ int unregister_console(struct console *console)
console_unlock();
return res;
}
+
+int unregister_console(struct console *console)
+{
+ int res;
+
+ console_list_lock();
+ res = unregister_console_locked(console);
+ console_list_unlock();
+ return res;
+}
EXPORT_SYMBOL(unregister_console);

/*
@@ -3414,6 +3472,7 @@ static int __init printk_late_init(void)
struct console *con;
int ret;

+ console_list_lock();
hlist_for_each_entry_safe(con, tmp, &console_list, node) {
if (!(con->flags & CON_BOOT))
continue;
@@ -3431,9 +3490,10 @@ static int __init printk_late_init(void)
*/
pr_warn("bootconsole [%s%d] uses init memory and must be disabled even before the real one is ready\n",
con->name, con->index);
- unregister_console(con);
+ unregister_console_locked(con);
}
}
+ console_list_unlock();

ret = cpuhp_setup_state_nocalls(CPUHP_PRINTK_DEAD, "printk:dead", NULL,
console_cpu_notify);
--
2.30.2