linux-kernel - two locations: was: Re: [PATCH printk v1 03/13] printk: use percpu flag instead of cpu

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: <Yg0C+UtoegnybA4q@alley>
Date:   Wed, 16 Feb 2022 14:58:17 +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>
Subject: two locations: was: Re: [PATCH printk v1 03/13] printk: use percpu
 flag instead of cpu_online()

On Mon 2022-02-07 20:49:13, John Ogness wrote:
> The CON_ANYTIME console flag is used to label consoles that will
> work correctly before percpu resources are allocated. To check
> the condition, cpu_online(raw_smp_processor_id()) was used.
> However, this is odd because CPUs can go offline at a later point.
> Also, the function is forced to use the raw_ variant because
> migration is not disabled.
> 
> Since commit ab6f762f0f53 ("printk: queue wake_up_klogd irq_work
> only if per-CPU areas are ready") there is a variable to identify
> if percpu resources have been allocated. Use that variable instead
> of cpu_online(raw_smp_processor_id()).

> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index d1b773823d63..b346e60e9e51 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2577,11 +2577,11 @@ static int have_callable_console(void)
>   *
>   * Console drivers may assume that per-cpu resources have been allocated. So
>   * unless they're explicitly marked as being able to cope (CON_ANYTIME) don't
> - * call them until this CPU is officially up.
> + * call them until per-cpu resources have been allocated.
>   */
>  static inline int can_use_console(void)
>  {
> -	return cpu_online(raw_smp_processor_id()) || have_callable_console();
> +	return (printk_percpu_data_ready() || have_callable_console());
>  }

cpu_online(raw_smp_processor_id()) check is used also in
call_console_drivers(). The same logic should be used in both
locations.

I found this when reviewing 6th patch that replaced both checks
with a single one.

Note that I am still not sure if this change is correct at all.
It will allow to always call the console during CPU hotplug
and I am not sure if it is safe. IMHO, it might cause problems when
a console driver uses, for example, CPU-bound workqueues.

Best Regards,
Petr