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

[Petr Mladek <pmladek@suse.com>]

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@chrisdown.name>

Looks good to me:

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

Best Regards,
Petr