linux-kernel - Re: [PATCH v4.2-rc1] printk: make extended printk support conditional on netconsole

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, 1 Jul 2015 18:05:13 +0200
From:	Petr Mladek <pmladek@...e.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Kay Sievers <kay@...y.org>,
	Josh Triplett <josh@...htriplett.org>,
	Linux Embedded <linux-embedded@...r.kernel.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>
Subject: Re: [PATCH v4.2-rc1] printk: make extended printk support
 conditional on netconsole

On Mon 2015-06-29 19:31:40, Tejun Heo wrote:
> 6fe29354befe ("printk: implement support for extended console
> drivers") implemented extended printk support for extended netconsole.
> The code added was miniscule but it added static 8k buffer
> unconditionally unnecessarily bloating the kernel for cases where
> extended netconsole is not used.
> 
> This patch introduces CONFIG_PRINTK_CON_EXTENDED which is selected by
> CONFIG_NETCONSOLE.  If the config option is not set, extended printk
> support is compiled out along with the static buffer.
> 
> Verified 8k reduction in vmlinux bss when !CONFIG_NETCONSOLE.
> 
> Signed-off-by: Tejun Heo <tj@...nel.org>
> Reported-and-suggested-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> ---
> Linus, Andrew.
> 
> This removes an unnecessary 8k bss bloat introduced during v4.2-rc1
> merge window on certain configs.  The original patch was routed
> through -mm.  How should this be routed?
> 
> Thanks.
> 
>  drivers/net/Kconfig    |    1 +
>  init/Kconfig           |    3 +++
>  kernel/printk/printk.c |   33 +++++++++++++++++++++++++++++----
>  3 files changed, 33 insertions(+), 4 deletions(-)

[...]

> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
[...]
> @@ -2561,9 +2584,11 @@ void register_console(struct console *ne
>  		console_drivers->next = newcon;
>  	}
>  
> -	if (newcon->flags & CON_EXTENDED)
> -		if (!nr_ext_console_drivers++)
> +	if (newcon->flags & CON_EXTENDED) {
> +		if (!nr_ext_console_drivers)
>  			pr_info("printk: continuation disabled due to ext consoles, expect more fragments in /dev/kmsg\n");
> +		inc_nr_ext_console_drivers();

We should handle also the situation when CON_EXTENDED is set
and CONFIG_PRINTK_CON_EXTENDED is not set by mistake. Otherwise,
we will not increment nr_ext_console_drivers here => ext_text will
not be filled in console_unlock() => call_console_drivers()
will print nothing for the CON_EXTENDED console.

At least, I would print an error here. Something like.

#ifndef CONFIG_PRINTK_CON_EXTENDED
		pr_err("The registered extended console will print nothing because the kernel is not compiled with PRINTK_CON_EXTENDED\n");
#endif

I wonder if there is a good identification of the console that can be printed.

Otherwise, it looks fine to me.

Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/