linux-kernel - Re: [PATCH printk v2 28/38] printk: console_unblank: use srcu console list iterator

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: <Y1gAkjvSS3XXUJly@alley>
Date:   Tue, 25 Oct 2022 17:28:18 +0200
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
Subject: Re: [PATCH printk v2 28/38] printk: console_unblank: use srcu
 console list iterator

On Wed 2022-10-19 17:01:50, John Ogness wrote:
> Use srcu console list iteration for console list traversal.
> 
> Document why the console_lock is still necessary.
> 
> Signed-off-by: John Ogness <john.ogness@...utronix.de>

Reviewed-by: Petr Mladek <pmladek@...e.com>

See a note below.

> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2948,10 +2948,14 @@ EXPORT_SYMBOL(console_conditional_schedule);
>  void console_unblank(void)
>  {
>  	struct console *c;
> +	int cookie;
>  
>  	/*
> -	 * console_unblank can no longer be called in interrupt context unless
> -	 * oops_in_progress is set to 1..
> +	 * Stop console printing because the unblank() callback may
> +	 * assume the console is not within its write() callback.

>From some reason, I believe this even without more details. I expect
that unbank() would need to refresh the screen and use similar
code as the write() callback.

It is different than the device() callback where the dependency is
less obvious.

Best Regards,
Petr

> +	 *
> +	 * If @oops_in_progress is set, this may be an atomic context.
> +	 * In that case, attempt a trylock as best-effort.
>  	 */
>  	if (oops_in_progress) {
>  		if (down_trylock_console_sem() != 0)
> @@ -2961,9 +2965,14 @@ void console_unblank(void)
>  
>  	console_locked = 1;
>  	console_may_schedule = 0;
> -	for_each_console(c)
> +
> +	cookie = console_srcu_read_lock();
> +	for_each_console_srcu(c) {
>  		if (console_is_enabled(c) && c->unblank)
>  			c->unblank();
> +	}
> +	console_srcu_read_unlock(cookie);
> +
>  	console_unlock();
>  
>  	if (!oops_in_progress)