linux-kernel - [RFC][PATCH 1/2] printk: lock console_sem before we unregister boot consoles

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: <20190423062511.1118-2-sergey.senozhatsky@gmail.com>
Date:   Tue, 23 Apr 2019 15:25:10 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: [RFC][PATCH 1/2] printk: lock console_sem before we unregister boot consoles

The following pattern is not completely safe:

     for_each_console(bcon)
         if (bcon->flags & CON_BOOT)
             unregister_console(bcon);

Because, in theory, console drivers list and console drivers
can be modified concurrently from another CPU. Take console_sem
lock, which protects console drivers list and console drivers,
before we start iterating console drivers list.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
---
 kernel/printk/printk.c | 98 ++++++++++++++++++++++++------------------
 1 file changed, 56 insertions(+), 42 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 17102fd4c136..b0e361ca1bea 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2605,6 +2605,48 @@ static int __init keep_bootcon_setup(char *str)
 
 early_param("keep_bootcon", keep_bootcon_setup);
 
+static int __unregister_console(struct console *console)
+{
+	struct console *a, *b;
+	int res;
+
+	pr_info("%sconsole [%s%d] disabled\n",
+		(console->flags & CON_BOOT) ? "boot" : "",
+		console->name, console->index);
+
+	res = _braille_unregister_console(console);
+	if (res)
+		return res;
+
+	res = 1;
+	if (console_drivers == console) {
+		console_drivers = console->next;
+		res = 0;
+	} else if (console_drivers) {
+		for (a = console_drivers->next, b = console_drivers;
+		     a; b = a, a = b->next) {
+			if (a == console) {
+				b->next = a->next;
+				res = 0;
+				break;
+			}
+		}
+	}
+
+	if (!res && (console->flags & CON_EXTENDED))
+		nr_ext_console_drivers--;
+
+	/*
+	 * If this isn't the last console and it has CON_CONSDEV set, we
+	 * need to set it on the next preferred console.
+	 */
+	if (console_drivers != NULL && console->flags & CON_CONSDEV)
+		console_drivers->flags |= CON_CONSDEV;
+
+	console->flags &= ~CON_ENABLED;
+	return res;
+}
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
@@ -2777,62 +2819,34 @@ void register_console(struct console *newcon)
 	pr_info("%sconsole [%s%d] enabled\n",
 		(newcon->flags & CON_BOOT) ? "boot" : "" ,
 		newcon->name, newcon->index);
-	if (bcon &&
-	    ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV) &&
-	    !keep_bootcon) {
-		/* We need to iterate through all boot consoles, to make
+
+	if (keep_bootcon)
+		return;
+
+	if (bcon && (newcon->flags & (CON_CONSDEV|CON_BOOT)) == CON_CONSDEV) {
+		console_lock();
+		/*
+		 * We need to iterate through all boot consoles, to make
 		 * sure we print everything out, before we unregister them.
 		 */
 		for_each_console(bcon)
 			if (bcon->flags & CON_BOOT)
-				unregister_console(bcon);
+				__unregister_console(bcon);
+		console_unlock();
+		console_sysfs_notify();
 	}
 }
 EXPORT_SYMBOL(register_console);
 
 int unregister_console(struct console *console)
 {
-        struct console *a, *b;
-	int res;
-
-	pr_info("%sconsole [%s%d] disabled\n",
-		(console->flags & CON_BOOT) ? "boot" : "" ,
-		console->name, console->index);
-
-	res = _braille_unregister_console(console);
-	if (res)
-		return res;
+	int ret;
 
-	res = 1;
 	console_lock();
-	if (console_drivers == console) {
-		console_drivers=console->next;
-		res = 0;
-	} else if (console_drivers) {
-		for (a=console_drivers->next, b=console_drivers ;
-		     a; b=a, a=b->next) {
-			if (a == console) {
-				b->next = a->next;
-				res = 0;
-				break;
-			}
-		}
-	}
-
-	if (!res && (console->flags & CON_EXTENDED))
-		nr_ext_console_drivers--;
-
-	/*
-	 * If this isn't the last console and it has CON_CONSDEV set, we
-	 * need to set it on the next preferred console.
-	 */
-	if (console_drivers != NULL && console->flags & CON_CONSDEV)
-		console_drivers->flags |= CON_CONSDEV;
-
-	console->flags &= ~CON_ENABLED;
+	ret = __unregister_console(console);
 	console_unlock();
 	console_sysfs_notify();
-	return res;
+	return ret;
 }
 EXPORT_SYMBOL(unregister_console);
 
-- 
2.21.0