linux-kernel - Re: [PATCH v4 18/20] x86: Convert to GENERIC

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:   Fri, 9 Apr 2021 12:20:49 +0200
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Rob Herring <robh@...nel.org>
Cc:     will@...nel.org, danielwa@...co.com,
        daniel@...pelevich.san-francisco.ca.us, arnd@...nel.org,
        akpm@...ux-foundation.org, linux-arch@...r.kernel.org,
        devicetree@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        microblaze <monstr@...str.eu>, linux-mips@...r.kernel.org,
        nios2 <ley.foon.tan@...el.com>, openrisc@...ts.librecores.org,
        linux-hexagon@...r.kernel.org, linux-riscv@...ts.infradead.org,
        x86@...nel.org, linux-xtensa@...ux-xtensa.org,
        linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCH v4 18/20] x86: Convert to GENERIC_CMDLINE



Le 08/04/2021 à 21:41, Rob Herring a écrit :
> On Fri, Apr 02, 2021 at 03:18:20PM +0000, Christophe Leroy wrote:
>> This converts the architecture to GENERIC_CMDLINE.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>> ---
>>   arch/x86/Kconfig        | 45 ++---------------------------------------
>>   arch/x86/kernel/setup.c | 17 ++--------------
>>   2 files changed, 4 insertions(+), 58 deletions(-)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index a20684d56b4b..66b384228ca3 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -104,6 +104,7 @@ config X86
>>   	select ARCH_USE_QUEUED_SPINLOCKS
>>   	select ARCH_USE_SYM_ANNOTATIONS
>>   	select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
>> +	select ARCH_WANT_CMDLINE_PREPEND_BY_DEFAULT
> 
> Seems to be non-existent kconfig option.

Oops. Added in v5.

>> @@ -883,18 +881,7 @@ void __init setup_arch(char **cmdline_p)
>>   	bss_resource.start = __pa_symbol(__bss_start);
>>   	bss_resource.end = __pa_symbol(__bss_stop)-1;
>>   
>> -#ifdef CONFIG_CMDLINE_BOOL
>> -#ifdef CONFIG_CMDLINE_FORCE
>> -	strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
>> -#else
>> -	if (builtin_cmdline[0]) {
>> -		/* append boot loader cmdline to builtin */
>> -		strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
>> -		strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
>> -		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
>> -	}
>> -#endif
>> -#endif
>> +	cmdline_build(boot_command_line, boot_command_line);
>>   
>>   	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
>>   	*cmdline_p = command_line;
> 
> Once this is all done, I wonder if we can get rid of the strlcpy and
> perhaps also cmdline_p.
> 

It seems rather complicated, in init/main.c you have heavy manipulations of command lines which 
seems to be done in setup_command_line() which seems to add stuff in front of command lines, at the 
end we end up with several command lines:

/* Untouched saved command line (eg. for /proc) */
char *saved_command_line;
/* Command line for parameter parsing */
static char *static_command_line;
/* Untouched extra command line */
static char *extra_command_line;

Some of them come from the cmdline_p which others are from boot_command_line.

I think a cleanup on all that stuff would be worth it as a further step.