linux-kernel - Re: [PATCH] printk: Move and rename maximum printk output line length defines

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:   Mon, 25 May 2020 17:38:45 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Chenggang Wang <wangchenggang@...o.com>,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] printk: Move and rename maximum printk output line
 length defines

On (20/05/21 11:40), Joe Perches wrote:
> Make the printk output maximum line length globally available.
> 
> This can be used to emit logging messages lines that exceed
> the single printk maximum length line.
> 
> e.g.: the kernel boot command on ARM can be up to 2048 chars

Speaking of the kernel boot command I still think that we can use
the existing approach here - split the command line and feed it
token by token to pr_cont(). printk() will handle everything
internally without exposing any implementation details (different
prefix sizes which depend on .config, etc. etc.) and requiring less
code.

> +/* Maximum length of a single printk */
> +
> +#ifdef CONFIG_PRINTK
> +
> +#ifdef CONFIG_PRINTK_CALLER
> +#define PRINTK_PREFIX_MAX	48
> +#else
> +#define PRINTK_PREFIX_MAX	32
> +#endif
> +#define PRINTK_LOG_LINE_MAX	(1024 - PRINTK_PREFIX_MAX)
> +
> +#else
> +
> +#define PRINTK_PREFIX_MAX	0
> +#define PRINTK_LOG_LINE_MAX	0
> +
> +#endif

!CONFIG_PRINTK case is concerning.

We depend on correct zero-sized LOG_LINE handling, otherwise things
like "p + PRINTK_LOG_LINE_MAX - 1" or "(p - line) < PRINTK_LOG_LINE_MAX - 1)"
are ticking bombs, waiting for !PRINTK config and off-by-one writes/reads.
Unless the code in question does more checks or is wrapped into
'#ifdef CONFIG_PRINTK', like you did in your print_cmdline() patch.

We may do a counter measure here, and simply undefine PRINTK_LOG_LINE_MAX
for !PRINTK config, thus forcing any code which uses PRINTK_LOG_LINE_MAX
to be wrapped into `#ifdef CONFIG_PRINTK`. But I think that pr_cont()-based
loop is safer and is easier to code.

If we set to implement "print any random very large string", then I'd
prefer to do it on the printk() side, just like was suggested by Andrew,
rather than forcing people to implement similar loops in various parts
of the kernel.

	-ss