Re: [PATCH 03/28] ARM: atags: Replace strlcat() with snprintf() for cmdline extend

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm: atags: OOB read and panic in parse_tag_cmdline() due to strlen()
--

commit 9d3172b712a6dbf6594b26ae8fb56ebd952c7d66
Author: Bill Wendling <morbo@google.com>

ARM: atags: Replace strlcat() with snprintf() for cmdline extend

This commit replaces the deprecated strlcat() API with snprintf() in
parse_tag_cmdline() to append the bootloader tag command line to
default_command_line.

Link: https://github.com/KSPP/linux/issues/370 [1]

> diff --git a/arch/arm/kernel/atags_parse.c b/arch/arm/kernel/atags_parse.c
> index 4ec591bde3dfa..21140b44d96e6 100644
> --- a/arch/arm/kernel/atags_parse.c
> +++ b/arch/arm/kernel/atags_parse.c
> @@ -121,9 +121,10 @@ __tagtable(ATAG_REVISION, parse_tag_revision);
>  static int __init parse_tag_cmdline(const struct tag *tag)
>  {
>  #if defined(CONFIG_CMDLINE_EXTEND)
> -	strlcat(default_command_line, " ", COMMAND_LINE_SIZE);
> -	strlcat(default_command_line, tag->u.cmdline.cmdline,
> -		COMMAND_LINE_SIZE);
> +	size_t len = strlen(default_command_line);

[Severity: High]
Is it possible for strlen() to read out of bounds or panic here?

If CONFIG_CMDLINE is exactly COMMAND_LINE_SIZE characters long, the C compiler
initializes the default_command_line array without a null terminator. The
original strlcat() code used strnlen() underneath and handled full buffers
safely.

Under CONFIG_FORTIFY_SOURCE, calling strlen() on an unterminated buffer triggers
a fortify_panic(), which will crash the kernel during early boot.

Without CONFIG_FORTIFY_SOURCE, strlen() will read out of bounds until it finds
a null byte, resulting in len being larger than COMMAND_LINE_SIZE.

> +
> +	snprintf(default_command_line + len, COMMAND_LINE_SIZE - len,
> +		 " %s", tag->u.cmdline.cmdline);

[Severity: High]
If len exceeds COMMAND_LINE_SIZE as described above, will this calculation
underflow?

This would cause COMMAND_LINE_SIZE - len to mathematically underflow to a huge
unsigned value.

When this huge size parameter is passed to snprintf(), it triggers a
WARN_ON_ONCE(size > INT_MAX) in vsnprintf().

>  #elif defined(CONFIG_CMDLINE_FORCE)
>  	pr_warn("Ignoring tag cmdline (using the default kernel command line)\n");

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915081910.4142719-1-morbo@google.com?part=3