[PATCHv2 1/4] printk: factor out __unregister_console() code

From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Petr Mladek <pmladek@suse.com>, Steven Rostedt <rostedt@goodmis.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: [PATCHv2 1/4] printk: factor out __unregister_console() code
Date: Fri, 26 Apr 2019 14:32:59 +0900	[thread overview]
Message-ID: <20190426053302.4332-2-sergey.senozhatsky@gmail.com> (raw)
In-Reply-To: <20190426053302.4332-1-sergey.senozhatsky@gmail.com>

The following pattern in register_console() 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. We need to grab
console_sem lock, which protects console drivers list and console
drivers, before we start iterating console drivers list.

Factor out __unregister_console(), which will be called from
unregister_console() and register_console(), in both cases
under console_sem lock.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.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

