linux-kernel - Re: console output duplicated when registering additional 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]

Date:   Thu, 14 Nov 2019 10:57:37 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Jonathan Richardson <jonathan.richardson@...adcom.com>
Cc:     gregkh@...uxfoundation.org, jslaby@...e.com,
        sergey.senozhatsky@...il.com, linux-serial@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Scott Branden <scott.branden@...adcom.com>,
        Ray Jui <ray.jui@...adcom.com>,
        Srinath Mannam <srinath.mannam@...adcom.com>
Subject: Re: console output duplicated when registering additional consoles

On Wed 2019-11-13 17:28:45, Jonathan Richardson wrote:
> Adding printk maintainers.
> This commit seems to have introduced the error:
> 
> commit f92b070f2dc89a8ff1a0cc8b608e20abef894c7d
> Author: Petr Mladek <pmladek@...e.com>
> Date:   Thu Sep 13 14:34:06 2018 +0200
> 
>     printk: Do not miss new messages when replaying the log
> 
> If I checkout to the commit before
> (a06b0c82a049d34d4dc273e8692ed0894458c468), the console output is
> normal when registering 2 bootconsoles and 2 normal consoles. I've
> added the log for 4.19.0-rc3 for comparison (previous version was
> 5.1.0). I don't think this commit took into account that more than one
> console could be registered. When the second console is registered,
> 'console_seq >= exclusive_console_stop_seq' is true (both are 0) and
> exclusive_console is always set to NULL resulting in the log being
> replayed again to the uart8250 console:

This race should not happen because Both exclusive_console and
exclusive_console_stop_seq are manipulated under console_lock.
And the log is replayed before console_lock is released.

>         /* Output to all consoles once old messages replayed. */
>         if (unlikely(exclusive_console &&
>                  console_seq >= exclusive_console_stop_seq)) {
>             exclusive_console = NULL;
>         }
> 
> I'm looking into it but any input is helpful. Thanks.

IMHO, the problem is that the log should not be replayed at all.
See the following code in register_console():

	/*
	 * If we have a bootconsole, and are switching to a real console,
	 * don't print everything out again, since when the boot console, and
	 * the real console are the same physical device, it's annoying to
	 * see the beginning boot messages twice
	 */
	if (bcon && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV))
		newcon->flags &= ~CON_PRINTBUFFER;

I already see two problems there:

1. CON_PRINTBUFFER is cleared only when the new console has
   CON_CONSDEV flag set. It is set only for the console
   that is defined as the last on the command line.
   It is a so-called preferred console.

2. bcon is set to the first console in console_drivers list.
   It is the first registered boot console.

Sigh, this works for simple configuration. But it fails badly when
more different consoles are configured.

We should clear CON_PRINTBUFFER flag when the real console
replacing an already registered boot console is registered.

BTW: Similar bug is also at the end of register_console().
The boot consoles are unregistered only when the preferred
console is installed.

For a proper solution we would need to match boot and real
consoles that write messages into the physical device.
But I am afraid that there is no support for this.
con->match() callback compares the name defined on
the command line. And it has side effects (the matching
console is prepared for registration).

To be honest I am not much familiar with the device interface.
I am not sure if there is a way to detect the two drivers
use the same physical hardware.

Sigh, it is a huge historical mess. It would needed a lot
of work to clean it up.

Best Regards,
Petr