Re: [PATCH v2 1/2] printk: Check valid console index for preferred console

[Greg Kroah-Hartman]

On Wed, Oct 11, 2023 at 10:43:25AM +0300, Tony Lindgren wrote:
> Let's check for valid console index values to avoid bogus console index
> numbers from kernel command line. While struct console uses short for
> index, and negative index values are used by some device drivers, we do
> not want to allow negative values for preferred console.

What drivers use a negative index for the console?

> Let's change the idx to short to match struct console, and return an error
> on negative values. And let's also constify idx while at it.
> 
> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
> ---
> 
> Changes since v1:
> 
> - Use const short idx and return an error on negative values
> 
> ---
>  include/linux/console.h |  2 +-
>  kernel/printk/printk.c  | 11 +++++++++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/console.h b/include/linux/console.h
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -340,7 +340,7 @@ enum con_flush_mode {
>  	CONSOLE_REPLAY_ALL,
>  };
>  
> -extern int add_preferred_console(char *name, int idx, char *options);
> +extern int add_preferred_console(char *name, const short idx, char *options);
>  extern void console_force_preferred_locked(struct console *con);
>  extern void register_console(struct console *);
>  extern int unregister_console(struct console *);
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2404,12 +2404,19 @@ static void set_user_specified(struct console_cmdline *c, bool user_specified)
>  	console_set_on_cmdline = 1;
>  }
>  
> -static int __add_preferred_console(char *name, int idx, char *options,
> +static int __add_preferred_console(const char *name, const short idx, char *options,
>  				   char *brl_options, bool user_specified)
>  {
>  	struct console_cmdline *c;
>  	int i;
>  
> +	/*
> +	 * Negative struct console index may be valid for drivers in some cases,
> +	 * but negative index is not valid for a preferred console.
> +	 */
> +	if (idx < 0)
> +		return -EINVAL;

Looks good to me, I'll take this through my tty tree if no one objects.

thanks,

greg k-h