Re: [PATCH v3 2/6] printk: add bounds checking to boot_delay

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The local variable `boot_delay_val` is read uninitialized when the `boot_delay` kernel parameter is passed without a value, leading to massive and unpredictable boot delays.
--

--- Patch [2]: [PATCH v3 2/6] printk: add bounds checking to boot_delay ---
commit d84630c2b1ca4df747144242cedfc7df2ff7c068
Author: Andrew Murray <[email protected]>
printk: add bounds checking to boot_delay

This commit changes the boot_delay parameter type to an unsigned int and
adds explicit bounds checking to ensure it remains within a valid range.

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 31aabdf8248cc..8be562c9be277 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
[ ... ]
>  static int __init boot_delay_setup(char *str)
>  {
>  	unsigned long lpj;
> +	int boot_delay_val;
>  
>  	lpj = preset_lpj ? preset_lpj : 1000000;	/* some guess */
>  	loops_per_msec = (unsigned long long)lpj / 1000 * HZ;
>  
> -	get_option(&str, &boot_delay);
> -	if (boot_delay > 10 * 1000)
> -		boot_delay = 0;
> +	get_option(&str, &boot_delay_val);
> +	if (boot_delay_val < 0 || boot_delay_val > 10 * 1000)
> +		return 0;
> +
> +	boot_delay = (unsigned int)boot_delay_val;

[Severity: High]
Does this code leave boot_delay_val uninitialized if get_option() returns
early?

When the boot_delay parameter is supplied without an assigned value (for
example, just passing "boot_delay" on the kernel command line), parse_args()
and do_early_param() pass a NULL string to boot_delay_setup().

In this scenario, get_option() returns 0 immediately without writing to
boot_delay_val.

If the uninitialized stack value happens to fall between 0 and 10000, it
gets assigned to the global boot_delay. Could this cause unexpected and
massive boot delays after every printk message?

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