linux-kernel - Re: [PATCH v7 05/13] printk: Add synchronisation for concurrent console state changes

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] [day] [month] [year] [list]

Message-ID: <aR33Kd6Voyba4CdE@pathway.suse.cz>
Date: Wed, 19 Nov 2025 17:58:17 +0100
From: Petr Mladek <pmladek@...e.com>
To: Chris Down <chris@...isdown.name>
Cc: linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	John Ogness <john.ogness@...utronix.de>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Tony Lindgren <tony.lindgren@...ux.intel.com>, kernel-team@...com
Subject: Re: [PATCH v7 05/13] printk: Add synchronisation for concurrent
 console state changes

On Wed 2025-11-19 03:07:17, Chris Down wrote:
> The syslog actions SYSLOG_ACTION_CONSOLE_OFF and
> SYSLOG_ACTION_CONSOLE_ON currently run without any locking. This creates
> a race condition if two processes attempt to toggle the console state
> simultaneously.
> 
> While this race has existed for donkey's years, it was previously
> somewhat tolerable because it involved only a single integer variable
> (saved_console_loglevel). However, upcoming changes will introduce a
> second piece of state (saved_ignore_per_console_loglevel) to be saved
> and restored. With two variables, this can result in saved state getting
> lost.
> 
> Here is a demonstration:
> 
>  CPU 0 (SYSLOG_ACTION_CONSOLE_OFF)        CPU 1 (SYSLOG_ACTION_CONSOLE_ON)
>  ---------------------------------        --------------------------------
>  // saved_console_loglevel is DEFAULT
>  if (saved == DEFAULT) (True)
>    saved = console_loglevel (7)
>                                           // Race triggers here
>                                           if (saved != DEFAULT) (True)
>                                             loglevel = saved (7)
>                                             saved = DEFAULT
>    // CPU 0 continues, unaware saved
>    // was just reset by CPU 1
>    loglevel = minimum_console_loglevel (1)
> 
> The result is that the console is now set to the minimum loglevel, but
> saved_console_loglevel is LOGLEVEL_DEFAULT. A subsequent CONSOLE_ON call
> will see saved == LOGLEVEL_DEFAULT and thus refuse to restore the
> original loglevel. The console is effectively stuck in quiet mode until
> manually reset via sysctl. Oh dear.
> 
> The callers of do_syslog are syscalls (so user context) or procfs write
> handlers. These contexts are allowed to sleep, so acquiring a mutex is
> safe. We also already make use of syslog_lock in
> SYSLOG_ACTION_SIZE_UNREAD and SYSLOG_ACTION_READ, so this is consistent.
> 
> Signed-off-by: Chris Down <chris@...isdown.name>

Looks good to me:

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

Best Regards,
Petr