linux-kernel - Re: [PATCHv2 1/4] printk: factor out __unregister

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:   Wed, 15 May 2019 15:34:52 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCHv2 1/4] printk: factor out __unregister_console() code

On Fri 2019-04-26 14:32:59, Sergey Senozhatsky wrote:
> The following pattern in register_console() is not completely safe:
> 
>      for_each_console(bcon)
>          if (bcon->flags & CON_BOOT)
>              unregister_console(bcon);
> 
> Because, in theory, console drivers list and console drivers
> can be modified concurrently from another CPU. We need to grab
> console_sem lock, which protects console drivers list and console
> drivers, before we start iterating console drivers list.
> 
> Factor out __unregister_console(), which will be called from
> unregister_console() and register_console(), in both cases
> under console_sem lock.
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 17102fd4c136..b0e361ca1bea 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2605,6 +2605,48 @@ static int __init keep_bootcon_setup(char *str)
>  
>  early_param("keep_bootcon", keep_bootcon_setup);
>  
> +static int __unregister_console(struct console *console)
> +{
> +	struct console *a, *b;
> +	int res;
> +
> +	pr_info("%sconsole [%s%d] disabled\n",
> +		(console->flags & CON_BOOT) ? "boot" : "",
> +		console->name, console->index);
> +
> +	res = _braille_unregister_console(console);

It looks safe to call _braille_unregister_console() under
console_lock().

Therefore this patch looks fine and make sense, so:

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

Best Regards,
Petr