Re: [PATCH 1/3] init/Kconfig: generalize config COMMAND_LINE_SIZE to all architectures

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Moving COMMAND_LINE_SIZE to init/Kconfig removes the minimum size limit of 896 bytes for s390, allowing legacy bootloaders to overflow the parmarea and corrupt the kernel .text section.
- [Medium] The newly introduced COMMAND_LINE_SIZE Kconfig option is silently bypassed and acts as a placebo for all architectures except s390.
- [Medium] Making COMMAND_LINE_SIZE globally configurable via Kconfig conflicts with its static definition in userspace-exported UAPI headers.
- [Medium] The Kconfig default definition for COMMAND_LINE_SIZE references the non-existent symbol 'UM'.
- [Medium] The default value of CONFIG_COMMAND_LINE_SIZE for RISC-V is incorrectly set to 1024, which is a reduction from its existing hardcoded size of 2048.
--

--- Patch [1]: [PATCH 1/3] init/Kconfig: generalize config COMMAND_LINE_SIZE to all architectures ---
commit 331242b120be7dde8a2decc92398462a27944ed5
Author: Wilson Felipe Pereira <[email protected]>
Subject: init/Kconfig: generalize config COMMAND_LINE_SIZE to all architectures

This commit moves CONFIG_COMMAND_LINE_SIZE from s390 to init/Kconfig to
allow all architectures to configure their maximum command line length. It
also sets default values intended to match the existing hardcoded sizes.

[ ... ]

> diff --git a/init/Kconfig b/init/Kconfig
> index 5230d4879b1c8..c581817966ee2 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1566,6 +1566,18 @@ config BOOT_CONFIG_EMBED_FILE
>  	  This bootconfig will be used if there is no initrd or no other
>  	  bootconfig in the initrd.
>  
> +config COMMAND_LINE_SIZE

[Severity: Medium]
Will this Kconfig option actually be used by other architectures?

For example, in arch/x86/include/asm/setup.h, COMMAND_LINE_SIZE is still
defined as a hardcoded macro:

#define COMMAND_LINE_SIZE 2048

Because the C preprocessor will continue to use the hardcoded macro rather
than the CONFIG_COMMAND_LINE_SIZE Kbuild variable, won't any changes made
by users in menuconfig be silently ignored on architectures other than s390?


[Severity: Medium]
Furthermore, does making this globally configurable conflict with its static
definition in userspace-exported UAPI headers?

Many architectures expose this as a hardcoded constant in their
include/uapi/asm/setup.h headers:

arch/arm64/include/uapi/asm/setup.h:
#define COMMAND_LINE_SIZE 2048

Since UAPI headers cannot depend on kernel build configurations (they do
not include <generated/autoconf.h>), if an architecture maintainer tries to
fix the bypass by replacing the hardcoded value with the Kconfig variable,
wouldn't that break the userspace ABI for tools like kexec-tools which rely
on the UAPI header?

> +	int "Maximum size of kernel command line"
> +	default 4096 if S390 || LOONGARCH || MIPS || UM

[Severity: Medium]
Is UM the correct Kconfig symbol here?

The User-Mode Linux architecture uses the config symbol UML. If UM
evaluates to false because it does not exist, won't User-Mode Linux builds
silently fall back to the incorrect default of 512 instead of 4096?

> +	default 2048 if X86 || ARM64 || POWERPC || SPARC64
> +	default 1024 if ARM || PARISC || RISCV

[Severity: Medium]
The commit message mentions adding defaults to keep values unchanged, but
is 1024 the correct default for RISC-V?

Checking arch/riscv/include/uapi/asm/setup.h shows that the current size
is 2048. If this Kconfig option is respected, would this cause a regression
by halving the allowed command line size for RISC-V?

> +	default 256 if ALPHA || ARC || M68K || MICROBLAZE || SPARC32 || XTENSA
> +	default 512
> +	range 256 1048576

[Severity: High]
Does lowering the minimum range to 256 introduce memory corruption risks
for s390?

The previous range in arch/s390/Kconfig enforced a minimum of 896 bytes. On
s390, the command_line buffer is placed at the end of struct parmarea, and
the executable .text section immediately follows it in the linker script:

arch/s390/boot/vmlinux.lds.S:
.text : {
    _text = .;  /* Text */

If an administrator configures a small size like 256, and a legacy
bootloader writes up to the historical 896 bytes into the parmarea,
wouldn't it overrun the buffer and corrupt the .text section?

> +	help
> +	  This allows you to specify the maximum length of the kernel command
> +	  line.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1