All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Juergen Gross <jgross@suse.com>,
	linux-fbdev@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Helge Deller <deller@gmx.de>,
	linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	Javier Martinez Canillas <javierm@redhat.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	dri-devel@lists.freedesktop.org,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Tom Rix <trix@redhat.com>, Thomas Gleixner <tglx@linutronix.de>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: [PATCH printk v5 32/40] printk, xen: fbfront: create/use safe function for forcing preferred
Date: Wed, 16 Nov 2022 17:27:44 +0106	[thread overview]
Message-ID: <20221116162152.193147-33-john.ogness@linutronix.de> (raw)
In-Reply-To: <20221116162152.193147-1-john.ogness@linutronix.de>

With commit 9e124fe16ff2("xen: Enable console tty by default in domU
if it's not a dummy") a hack was implemented to make sure that the
tty console remains the console behind the /dev/console device. The
main problem with the hack is that, after getting the console pointer
to the tty console, it is assumed the pointer is still valid after
releasing the console_sem. This assumption is incorrect and unsafe.

Make the hack safe by introducing a new function
console_force_preferred_locked() and perform the full operation
under the console_list_lock.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 drivers/video/fbdev/xen-fbfront.c | 12 +++-----
 include/linux/console.h           |  1 +
 kernel/printk/printk.c            | 49 +++++++++++++++++++++++++++++--
 3 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 4d2694d904aa..8752d389e382 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -504,18 +504,14 @@ static void xenfb_make_preferred_console(void)
 	if (console_set_on_cmdline)
 		return;
 
-	console_lock();
+	console_list_lock();
 	for_each_console(c) {
 		if (!strcmp(c->name, "tty") && c->index == 0)
 			break;
 	}
-	console_unlock();
-	if (c) {
-		unregister_console(c);
-		c->flags |= CON_CONSDEV;
-		c->flags &= ~CON_PRINTBUFFER; /* don't print again */
-		register_console(c);
-	}
+	if (c)
+		console_force_preferred_locked(c);
+	console_list_unlock();
 }
 
 static int xenfb_resume(struct xenbus_device *dev)
diff --git a/include/linux/console.h b/include/linux/console.h
index f716e1dd9eaf..9cea254b34b8 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -291,6 +291,7 @@ enum con_flush_mode {
 };
 
 extern int add_preferred_console(char *name, int idx, char *options);
+extern void console_force_preferred_locked(struct console *con);
 extern void register_console(struct console *);
 extern int unregister_console(struct console *);
 extern void console_lock(void);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 410d3f2cdeb3..ece34abbc9cc 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -247,9 +247,10 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
 void console_list_lock(void)
 {
 	/*
-	 * 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.
+	 * In unregister_console() and console_force_preferred_locked(),
+	 * 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.
 	 *
 	 * Detecting if this context is really in the read-side critical
 	 * section is only possible if the appropriate debug options are
@@ -3489,6 +3490,48 @@ int unregister_console(struct console *console)
 }
 EXPORT_SYMBOL(unregister_console);
 
+/**
+ * console_force_preferred_locked - force a registered console preferred
+ * @con: The registered console to force preferred.
+ *
+ * Must be called under console_list_lock().
+ */
+void console_force_preferred_locked(struct console *con)
+{
+	struct console *cur_pref_con;
+
+	if (!console_is_registered_locked(con))
+		return;
+
+	cur_pref_con = console_first();
+
+	/* Already preferred? */
+	if (cur_pref_con == con)
+		return;
+
+	/*
+	 * Delete, but do not re-initialize the entry. This allows the console
+	 * to continue to appear registered (via any hlist_unhashed_lockless()
+	 * checks), even though it was briefly removed from the console list.
+	 */
+	hlist_del_rcu(&con->node);
+
+	/*
+	 * Ensure that all SRCU list walks have completed so that the console
+	 * can be added to the beginning of the console list and its forward
+	 * list pointer can be re-initialized.
+	 */
+	synchronize_srcu(&console_srcu);
+
+	con->flags |= CON_CONSDEV;
+	WARN_ON(!con->device);
+
+	/* Only the new head can have CON_CONSDEV set. */
+	console_srcu_write_flags(cur_pref_con, cur_pref_con->flags & ~CON_CONSDEV);
+	hlist_add_head_rcu(&con->node, &console_list);
+}
+EXPORT_SYMBOL(console_force_preferred_locked);
+
 /*
  * Initialize the console device. This is called *early*, so
  * we can't necessarily depend on lots of kernel help here.
-- 
2.30.2


WARNING: multiple messages have this Message-ID (diff)
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org, Helge Deller <deller@gmx.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Javier Martinez Canillas <javierm@redhat.com>,
	Juergen Gross <jgross@suse.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Tom Rix <trix@redhat.com>,
	linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: [PATCH printk v5 32/40] printk, xen: fbfront: create/use safe function for forcing preferred
Date: Wed, 16 Nov 2022 17:27:44 +0106	[thread overview]
Message-ID: <20221116162152.193147-33-john.ogness@linutronix.de> (raw)
In-Reply-To: <20221116162152.193147-1-john.ogness@linutronix.de>

With commit 9e124fe16ff2("xen: Enable console tty by default in domU
if it's not a dummy") a hack was implemented to make sure that the
tty console remains the console behind the /dev/console device. The
main problem with the hack is that, after getting the console pointer
to the tty console, it is assumed the pointer is still valid after
releasing the console_sem. This assumption is incorrect and unsafe.

Make the hack safe by introducing a new function
console_force_preferred_locked() and perform the full operation
under the console_list_lock.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 drivers/video/fbdev/xen-fbfront.c | 12 +++-----
 include/linux/console.h           |  1 +
 kernel/printk/printk.c            | 49 +++++++++++++++++++++++++++++--
 3 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 4d2694d904aa..8752d389e382 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -504,18 +504,14 @@ static void xenfb_make_preferred_console(void)
 	if (console_set_on_cmdline)
 		return;
 
-	console_lock();
+	console_list_lock();
 	for_each_console(c) {
 		if (!strcmp(c->name, "tty") && c->index == 0)
 			break;
 	}
-	console_unlock();
-	if (c) {
-		unregister_console(c);
-		c->flags |= CON_CONSDEV;
-		c->flags &= ~CON_PRINTBUFFER; /* don't print again */
-		register_console(c);
-	}
+	if (c)
+		console_force_preferred_locked(c);
+	console_list_unlock();
 }
 
 static int xenfb_resume(struct xenbus_device *dev)
diff --git a/include/linux/console.h b/include/linux/console.h
index f716e1dd9eaf..9cea254b34b8 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -291,6 +291,7 @@ enum con_flush_mode {
 };
 
 extern int add_preferred_console(char *name, int idx, char *options);
+extern void console_force_preferred_locked(struct console *con);
 extern void register_console(struct console *);
 extern int unregister_console(struct console *);
 extern void console_lock(void);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 410d3f2cdeb3..ece34abbc9cc 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -247,9 +247,10 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
 void console_list_lock(void)
 {
 	/*
-	 * 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.
+	 * In unregister_console() and console_force_preferred_locked(),
+	 * 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.
 	 *
 	 * Detecting if this context is really in the read-side critical
 	 * section is only possible if the appropriate debug options are
@@ -3489,6 +3490,48 @@ int unregister_console(struct console *console)
 }
 EXPORT_SYMBOL(unregister_console);
 
+/**
+ * console_force_preferred_locked - force a registered console preferred
+ * @con: The registered console to force preferred.
+ *
+ * Must be called under console_list_lock().
+ */
+void console_force_preferred_locked(struct console *con)
+{
+	struct console *cur_pref_con;
+
+	if (!console_is_registered_locked(con))
+		return;
+
+	cur_pref_con = console_first();
+
+	/* Already preferred? */
+	if (cur_pref_con == con)
+		return;
+
+	/*
+	 * Delete, but do not re-initialize the entry. This allows the console
+	 * to continue to appear registered (via any hlist_unhashed_lockless()
+	 * checks), even though it was briefly removed from the console list.
+	 */
+	hlist_del_rcu(&con->node);
+
+	/*
+	 * Ensure that all SRCU list walks have completed so that the console
+	 * can be added to the beginning of the console list and its forward
+	 * list pointer can be re-initialized.
+	 */
+	synchronize_srcu(&console_srcu);
+
+	con->flags |= CON_CONSDEV;
+	WARN_ON(!con->device);
+
+	/* Only the new head can have CON_CONSDEV set. */
+	console_srcu_write_flags(cur_pref_con, cur_pref_con->flags & ~CON_CONSDEV);
+	hlist_add_head_rcu(&con->node, &console_list);
+}
+EXPORT_SYMBOL(console_force_preferred_locked);
+
 /*
  * Initialize the console device. This is called *early*, so
  * we can't necessarily depend on lots of kernel help here.
-- 
2.30.2


  parent reply	other threads:[~2022-11-16 16:22 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-16 16:21 [PATCH printk v5 00/40] reduce console_lock scope John Ogness
2022-11-16 16:21 ` John Ogness
2022-11-16 16:21 ` John Ogness
2022-11-16 16:21 ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 01/40] serial: kgdboc: Lock console list in probe function John Ogness
2022-11-16 16:21 ` [PATCH printk v5 02/40] printk: Convert console_drivers list to hlist John Ogness
2022-11-16 16:21 ` [PATCH printk v5 03/40] printk: Prepare for SRCU console list protection John Ogness
2022-12-01 18:22   ` Nathan Chancellor
2022-12-01 21:36     ` John Ogness
2022-12-01 21:56       ` Paul E. McKenney
2022-12-01 22:12         ` John Ogness
2022-12-02  0:21           ` Paul E. McKenney
2022-12-02 10:53             ` Petr Mladek
2022-11-16 16:21 ` [PATCH printk v5 04/40] printk: register_console: use "registered" for variable names John Ogness
2022-11-16 16:21 ` [PATCH printk v5 05/40] printk: move @seq initialization to helper John Ogness
2022-11-18  9:56   ` Petr Mladek
2022-11-16 16:21 ` [PATCH printk v5 06/40] printk: fix setting first seq for consoles John Ogness
2022-11-18 10:23   ` Petr Mladek
2022-11-18 10:52     ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 07/40] um: kmsg_dump: only dump when no output console available John Ogness
2022-11-16 16:21   ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 08/40] tty: serial: kgdboc: document console_lock usage John Ogness
2022-11-16 16:21 ` [PATCH printk v5 09/40] tty: tty_io: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 10/40] proc: consoles: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 11/40] printk: introduce console_list_lock John Ogness
2022-11-21 11:10   ` [PATCH printk v6 " John Ogness
2022-11-21 13:36     ` Petr Mladek
2022-11-16 16:21 ` [PATCH printk v5 12/40] console: introduce wrappers to read/write console flags John Ogness
2022-11-16 16:21 ` [PATCH printk v5 13/40] um: kmsg_dumper: use srcu console list iterator John Ogness
2022-11-16 16:21   ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 14/40] kdb: " John Ogness
2022-11-17  0:59   ` Doug Anderson
2022-11-16 16:21 ` [PATCH printk v5 15/40] printk: console_flush_all: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 16/40] printk: __pr_flush: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 17/40] printk: console_is_usable: use console_srcu_read_flags John Ogness
2022-11-16 16:21 ` [PATCH printk v5 18/40] printk: console_unblank: use srcu console list iterator John Ogness
2022-11-16 16:21 ` [PATCH printk v5 19/40] printk: console_flush_on_panic: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 20/40] printk: console_device: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 21/40] console: introduce console_is_registered() John Ogness
2022-11-16 16:21 ` [PATCH printk v5 22/40] serial_core: replace uart_console_enabled() with uart_console_registered() John Ogness
2022-11-16 16:21 ` [PATCH printk v5 23/40] tty: nfcon: use console_is_registered() John Ogness
2022-11-17  8:18   ` Geert Uytterhoeven
2022-11-16 16:21 ` [PATCH printk v5 24/40] efi: earlycon: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 25/40] tty: hvc: " John Ogness
2022-11-16 16:21   ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 26/40] tty: serial: earlycon: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 27/40] tty: serial: pic32_uart: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 28/40] tty: serial: samsung_tty: " John Ogness
2022-11-16 16:21   ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 29/40] tty: serial: xilinx_uartps: " John Ogness
2022-11-16 16:21   ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 30/40] usb: early: xhci-dbc: " John Ogness
2022-11-16 16:21 ` [PATCH printk v5 31/40] netconsole: avoid CON_ENABLED misuse to track registration John Ogness
2022-11-16 16:21 ` John Ogness [this message]
2022-11-16 16:21   ` [PATCH printk v5 32/40] printk, xen: fbfront: create/use safe function for forcing preferred John Ogness
2022-11-16 16:21 ` [PATCH printk v5 33/40] tty: tty_io: use console_list_lock for list synchronization John Ogness
2022-11-16 16:21 ` [PATCH printk v5 34/40] proc: consoles: use console_list_lock for list iteration John Ogness
2022-11-16 16:21 ` [PATCH printk v5 35/40] tty: serial: kgdboc: use srcu console list iterator John Ogness
2022-11-17  0:59   ` Doug Anderson
2022-11-17  9:32     ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 36/40] tty: serial: kgdboc: use console_list_lock for list traversal John Ogness
2022-11-17  0:59   ` Doug Anderson
2022-11-16 16:21 ` [PATCH printk v5 37/40] tty: serial: kgdboc: synchronize tty_find_polling_driver() and register_console() John Ogness
2022-11-17  0:59   ` Doug Anderson
2022-11-17 10:00     ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 38/40] tty: serial: kgdboc: use console_list_lock to trap exit John Ogness
2022-11-17  0:56   ` Doug Anderson
2022-11-17 11:08     ` John Ogness
2022-11-16 16:21 ` [PATCH printk v5 39/40] printk: relieve console_lock of list synchronization duties John Ogness
2022-11-18 11:07   ` Petr Mladek
2022-11-16 16:21 ` [PATCH printk v5 40/40] tty: serial: sh-sci: use setup() callback for early console John Ogness
2022-11-18 11:22 ` [PATCH printk v5 00/40] reduce console_lock scope Petr Mladek
2022-11-18 11:22   ` Petr Mladek
2022-11-18 11:22   ` Petr Mladek
2022-11-18 11:22   ` Petr Mladek
2022-11-18 14:55   ` Petr Mladek
2022-11-18 14:55     ` Petr Mladek
2022-11-18 14:55     ` Petr Mladek
2022-11-18 14:55     ` Petr Mladek
2022-11-22 16:43 ` Greg Kroah-Hartman
2022-11-22 16:43   ` Greg Kroah-Hartman
2022-11-22 16:43   ` Greg Kroah-Hartman
2022-11-22 16:43   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221116162152.193147-33-john.ogness@linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=boris.ostrovsky@oracle.com \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=javierm@redhat.com \
    --cc=jgross@suse.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=tglx@linutronix.de \
    --cc=trix@redhat.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.