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

Greg Kroah-Hartman
2023-10-11T07:53:23Z

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@atomide.com>
> ---
>
> 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