Re: [PATCH v2 1/4] printk: remove BOOT_PRINTK_DELAY config option

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The kernel command line parameter `boot_delay` can be parsed as a negative integer, which causes a massive integer underflow and a near-infinite loop, resulting in a boot hang.
--

commit 7ede3add142d62f673ea88e35c108d0ec8e7599d
Author: Andrew Murray <amurray@xxxxxxxxxxxxxxxxxxxx>

printk: remove BOOT_PRINTK_DELAY config option

The boot_delay (BOOT_PRINTK_DELAY) kernel parameter and printk_delay sysctl
are two distinct mechanisms for providing similar functionality which add a
delay prior to each printed printk message. This commit prepares to combine
them by removing the BOOT_PRINTK_DELAY kconfig option.

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2fe9a963c823a..77f53eaed1321 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1289,7 +1289,7 @@ static bool suppress_message_printing(int level)
>  	return (level >= console_loglevel && !ignore_loglevel);
>  }
>  
> -#ifdef CONFIG_BOOT_PRINTK_DELAY
> +#ifdef CONFIG_GENERIC_CALIBRATE_DELAY
>  
>  static int boot_delay; /* msecs delay after each printk during bootup */
>  static unsigned long long loops_per_msec;	/* based on boot_delay */

[Severity: High]
This is a pre-existing issue, but does expanding the availability of the
boot_delay parameter expose a potential boot hang if a negative value is
provided on the kernel command line?

When boot_delay is parsed in boot_delay_setup(), the bounds check does not
reject negative inputs:

kernel/printk/printk.c:boot_delay_setup() {
    ...
    get_option(&str, &boot_delay);
    if (boot_delay > 10 * 1000)
        boot_delay = 0;
    ...
}

If a user provides a negative value like boot_delay=-1, could
boot_delay_msec() cast this negative integer to an unsigned type, resulting
in an immense timeout?

kernel/printk/printk.c:boot_delay_msec() {
    ...
    k = (unsigned long long)loops_per_msec * boot_delay;

    timeout = jiffies + msecs_to_jiffies(boot_delay);
    ...
}

Because msecs_to_jiffies() casts the negative value to an unsigned int, this
could result in spinning in cpu_relax() almost indefinitely.

Might it be safer to add a check in boot_delay_setup() to ensure boot_delay
is not less than zero?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-deprecate_boot_delay-v2-0-f9883d36aa4b@xxxxxxxxxxxxxxxxxxxx?part=1