linux-kernel - Re: [RFC PATCH 2/2] init: Allow multi-line output of kernel command line

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:   Thu, 21 May 2020 11:59:31 -0400
From:   Arvind Sankar <nivedita@...m.mit.edu>
To:     Joe Perches <joe@...ches.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Chenggang Wang <wangchenggang@...o.com>,
        linux-kernel@...r.kernel.org, Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [RFC PATCH 2/2] init: Allow multi-line output of kernel command
 line

On Tue, May 19, 2020 at 12:42:35PM -0700, Joe Perches wrote:
> ARM may have its longest possible command line larger than the longest
> possible printk.
> 
> If necessary, emit the commend line on multiple lines.
> 
> Signed-off-by: Joe Perches <joe@...ches.com>
> ---
> 
> compiled, untested
> 
>  init/main.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/init/main.c b/init/main.c
> index b63a3c001ac4..b3ebbbc129ae 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -826,6 +826,34 @@ void __init __weak arch_call_rest_init(void)
>  	rest_init();
>  }
>  
> +static void __init print_cmdline(char *line)
> +{
> +#ifdef CONFIG_PRINTK
> +	const char *prefix = "Kernel command line";
> +	size_t len = strlen(line);
> +
> +	while (len > PRINTK_LOG_LINE_MAX) {
> +		char *pos = line;
> +		char *last_pos = pos + PRINTK_LOG_LINE_MAX - 1;
> +		char saved_char;
> +		/* Find last space char within the maximum line length */
> +		while ((pos = memchr(pos, ' ', len - (pos - line))) &&
> +		       (pos - line) < PRINTK_LOG_LINE_MAX - 1) {
> +			last_pos = pos;
> +		}
> +		saved_char = line[last_pos - line];
> +		line[last_pos - line] = 0;
> +		pr_notice("%s: %s\n", prefix, line);
> +		prefix = "Kernel command line (continued)";
> +		line[last_pos - line] = saved_char;
> +		len -= pos - line;
> +		line += pos - line;
> +	}
> +
> +	pr_notice("%s: %s\n", prefix, line);
> +#endif

I might be missing something, but this seems broken:
(1) If there is a ' ', the memchr will set pos to the ' ' the first time
through the inner loop, and then go into an infinite loop? You want
memrchr here but the kernel doesn't seem to have one.
(2) If there are no remaining ' 's pos will be NULL and the last two
lines in the outer loop use it -- those should be last_pos instead?
(3) Once those are fixed, you need to ensure you make progress: if
there's exactly one ' ' remaining, at the very beginning of the
remaining line, you need to print upto the PRINTK_LOG_LINE_MAX, not 0
bytes.