linux-kernel - [PATCH 1/1] kernel: Match preferred console first.

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-next>] [day] [month] [year] [list]
Message-Id: <20230213113912.1237943-1-rkanwal@rivosinc.com>
Date:   Mon, 13 Feb 2023 11:39:12 +0000
From:   Rajnesh Kanwal <rkanwal@...osinc.com>
To:     rkanwal@...osinc.com
Cc:     jirislaby@...nel.org, pmladek@...e.com, senozhatsky@...omium.org,
        rostedt@...dmis.org, john.ogness@...utronix.de,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: [PATCH 1/1] kernel: Match preferred console first.

For cases where the driver registers the console with the index set to -1
the `try_enable_preferred_console()` doesn't work as expected for multiple
hvc consoles.

For example with `CONFIG_HVC_RISCV_SBI` and `CONFIG_VIRTIO_CONSOLE` enabled
and cmdline set to "console=hvc0 console=hvc1" where hvc0 being RISCV_SBI
and hvc1 being VIRTIO_CONSOLE (the preferred one as per the preferred
selection logic in `__add_preferred_console`). hvc1 never gets enabled as hvc0
matches first with the driver name "hvc" and index set to -1.

Here is the hvc console structure from hvc_console.c L217.
static struct console hvc_console = {
        .name           = "hvc",
        .write          = hvc_console_print,
        .device         = hvc_console_device,
        .setup          = hvc_console_setup,
        .flags          = CON_PRINTBUFFER,
        .index          = -1,
};

The expected flow is that hvc1 should take precedence but given hvc console has
the index set to -1, the current logic will try to assign the first console that
matches, which is hvc0 and return. Never touching other possible consoles.

This change makes sure the preferred console is checked first and then iterate
the remaining list.

Signed-off-by: Rajnesh Kanwal <rkanwal@...osinc.com>
---
 drivers/tty/hvc/hvc_console.c |  2 +-
 kernel/printk/printk.c        | 78 ++++++++++++++++++++++++-----------
 2 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index a683e21df19c..f05bb92d9419 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -230,7 +230,7 @@ static struct console hvc_console = {
  * -- index will remain -1
  * (2) we are first and the user specified no driver
  * -- index will be set to 0, then we will fail setup.
- * (3)  we are first and the user specified our driver
+ * (3) we are first and the user specified our driver
  * -- index will be set to user specified driver, and we will fail
  * (4) we are after driver, and this initcall will register us
  * -- if the user didn't specify a driver then the console will match
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a5ed2e53547c..3be3ace255a9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3148,6 +3148,49 @@ static int __init keep_bootcon_setup(char *str)

 early_param("keep_bootcon", keep_bootcon_setup);

+/* Tries to enable newcon console device matching it with passed in
+ * console_cmdline. In case if match fails the function return -EAGAIN
+ * meaning the caller should try with the next console_cmdline.
+ */
+static int __try_enable_console(struct console *newcon,
+				struct console_cmdline *c, bool user_specified,
+				bool preferred)
+{
+	int err;
+
+	if (c->user_specified != user_specified)
+		return -EAGAIN;
+
+	if (!newcon->match ||
+	    newcon->match(newcon, c->name, c->index, c->options) != 0) {
+		/* default matching */
+		BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
+		if (strcmp(c->name, newcon->name) != 0)
+			return -EAGAIN;
+
+		if (newcon->index >= 0 && newcon->index != c->index)
+			return -EAGAIN;
+
+		if (newcon->index < 0)
+			newcon->index = c->index;
+
+		if (_braille_register_console(newcon, c))
+			return 0;
+
+		if (newcon->setup &&
+		    (err = newcon->setup(newcon, c->options)) != 0)
+			return err;
+
+		newcon->flags |= CON_ENABLED;
+		if (preferred)
+			newcon->flags |= CON_CONSDEV;
+
+		return 0;
+	}
+
+	return -EAGAIN;
+}
+
 /*
  * This is called by register_console() to try to match
  * the newly registered console with any of the ones selected
@@ -3163,34 +3206,23 @@ static int try_enable_preferred_console(struct console *newcon,
 	struct console_cmdline *c;
 	int i, err;

+	/* Try to match preferred console first. */
+	if (preferred_console >= 0) {
+		c = &console_cmdline[preferred_console];
+		err = __try_enable_console(newcon, c, user_specified, true);
+		if (err != EAGAIN)
+			return err;
+	}
+
 	for (i = 0, c = console_cmdline;
 	     i < MAX_CMDLINECONSOLES && c->name[0];
 	     i++, c++) {
-		if (c->user_specified != user_specified)
+		if (i == preferred_console)
 			continue;
-		if (!newcon->match ||
-		    newcon->match(newcon, c->name, c->index, c->options) != 0) {
-			/* default matching */
-			BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
-			if (strcmp(c->name, newcon->name) != 0)
-				continue;
-			if (newcon->index >= 0 &&
-			    newcon->index != c->index)
-				continue;
-			if (newcon->index < 0)
-				newcon->index = c->index;
-
-			if (_braille_register_console(newcon, c))
-				return 0;

-			if (newcon->setup &&
-			    (err = newcon->setup(newcon, c->options)) != 0)
-				return err;
-		}
-		newcon->flags |= CON_ENABLED;
-		if (i == preferred_console)
-			newcon->flags |= CON_CONSDEV;
-		return 0;
+		err = __try_enable_console(newcon, c, user_specified, false);
+		if (err != EAGAIN)
+			return err;
 	}

 	/*
--
2.25.1