linux-kernel - global states: was: Re: [PATCH printk v1 05/18] printk: Add non-BKL console basic infrastructure

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, 9 Mar 2023 15:08:43 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-fsdevel@...r.kernel.org
Subject: global states: was: Re: [PATCH printk v1 05/18] printk: Add non-BKL
 console basic infrastructure

On Thu 2023-03-02 21:02:05, John Ogness wrote:
> From: Thomas Gleixner <tglx@...utronix.de>
> 
> The current console/printk subsystem is protected by a Big Kernel Lock,
> (aka console_lock) which has ill defined semantics and is more or less
> stateless. This puts severe limitations on the console subsystem and
> makes forced takeover and output in emergency and panic situations a
> fragile endavour which is based on try and pray.
> 
> The goal of non-BKL consoles is to break out of the console lock jail
> and to provide a new infrastructure that avoids the pitfalls and
> allows console drivers to be gradually converted over.
> 
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3472,6 +3492,14 @@ void register_console(struct console *newcon)
>  	newcon->dropped = 0;
>  	console_init_seq(newcon, bootcon_registered);
>  
> +	if (!(newcon->flags & CON_NO_BKL))
> +		have_bkl_console = true;

We never clear this value even when the console gets unregistered.

> +	else
> +		cons_nobkl_init(newcon);
> +
> +	if (newcon->flags & CON_BOOT)
> +		have_boot_console = true;
> +
>  	/*
>  	 * Put this console in the list - keep the
>  	 * preferred driver at the head of the list.
> @@ -3515,6 +3543,9 @@ void register_console(struct console *newcon)
>  			if (con->flags & CON_BOOT)
>  				unregister_console_locked(con);
>  		}
> +
> +		/* All boot consoles have been unregistered. */
> +		have_boot_console = false;

The boot consoles can be removed also by printk_late_init().

I would prefer to make this more error-proof and update both
have_bkl_console and have_boot_console in unregister_console().

A solution would be to use a reference counter instead of the boolean.
I am not sure if it is worth it. But it seems that refcount_read()
is just simple atomic read, aka READ_ONCE().


>  	}
>  unlock:
>  	console_list_unlock();

Best Regards,
Petr