linux-kernel - Re: [PATCH printk v3 39/40] printk: relieve console

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: <87r0ye916v.fsf@jogness.linutronix.de>
Date:   Mon, 07 Nov 2022 17:36:48 +0106
From:   John Ogness <john.ogness@...utronix.de>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH printk v3 39/40] printk: relieve console_lock of list
 synchronization duties

On 2022-11-07, John Ogness <john.ogness@...utronix.de> wrote:
> @@ -3344,7 +3340,6 @@ void register_console(struct console *newcon)
>  	 * Put this console in the list - keep the
>  	 * preferred driver at the head of the list.
>  	 */
> -	console_lock();
>  	if (hlist_empty(&console_list)) {
>  		/* Ensure CON_CONSDEV is always set for the head. */
>  		newcon->flags |= CON_CONSDEV;
> @@ -3358,7 +3353,6 @@ void register_console(struct console *newcon)
>  	} else {
>  		hlist_add_behind_rcu(&newcon->node, console_list.first);
>  	}
> -	console_unlock();
>  
>  	/*
>  	 * No need to synchronize SRCU here! The caller does not rely

I just realized that because of the new @seq initialization (patch 5/40)
that we cannot completely remove the console_lock from
register_console(). It will still be needed for @seq synchronization
when registering non-boot/non-printbuffer consoles. So something like
the patch below will need to be folded into this one.

I am not happy with this. If an enabled boot console is behind, the
console_unlock() will probably catch it up and we will end up with some
repeat messages. But maybe this is "good enough" until we implement some
real coordination between boot console and normal console takeovers.

John Ogness

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 17765166ac42..bb119001df56 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3328,12 +3328,21 @@ void register_console(struct console *newcon)
 		 * that message instead. That boot console will be
 		 * unregistered shortly and may be the same device.
 		 */
+
+		/*
+		 * Hold the console_lock to guarantee safe access to
+		 * console->seq.
+		 */
+		console_lock();
+
 		for_each_console(con) {
 			if ((con->flags & (CON_BOOT | CON_ENABLED)) == (CON_BOOT | CON_ENABLED) &&
 			    con->seq < newcon->seq) {
 				newcon->seq = con->seq;
 			}
 		}
+
+		console_unlock();
 	}
 
 	/*