linux-kernel - Re: [PATCH v2 2/3] serial: core: Add support for DEVNAME:0.0 style naming for kernel console

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c9c637a-9117-4f43-a64f-892fa33958c1@kernel.org>
Date:   Thu, 14 Sep 2023 07:43:31 +0200
From:   Jiri Slaby <jirislaby@...nel.org>
To:     Tony Lindgren <tony@...mide.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Andy Shevchenko <andriy.shevchenko@...el.com>,
        Dhruva Gole <d-gole@...com>,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        John Ogness <john.ogness@...utronix.de>,
        Johan Hovold <johan@...nel.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Vignesh Raghavendra <vigneshr@...com>,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH v2 2/3] serial: core: Add support for DEVNAME:0.0 style
 naming for kernel console

On 12. 09. 23, 13:03, Tony Lindgren wrote:
> --- /dev/null
> +++ b/drivers/tty/serial/serial_base_con.c
...
> +/* Adds a command line console to the list of consoles for driver probe time */
> +static int __init serial_base_add_con(char *name, char *opt)
> +{
> +	struct serial_base_console *con;
> +
> +	con = kzalloc(sizeof(*con), GFP_KERNEL);
> +	if (!con)
> +		return -ENOMEM;
> +
> +	con->name = kstrdup(name, GFP_KERNEL);
> +	if (!con->name)
> +		goto free_con;
> +
> +	if (opt) {
> +		con->opt = kstrdup(opt, GFP_KERNEL);
> +		if (!con->name)

con->opt

> +			goto free_name;
> +	}
> +
> +	list_add_tail(&con->node, &serial_base_consoles);
> +
> +	return 0;
> +
> +free_name:
> +	kfree(con->name);
> +
> +free_con:
> +	kfree(con);
> +
> +	return -ENOMEM;
> +}
> +
> +/* Parse console name and options */
> +static int __init serial_base_parse_one(char *param, char *val,
> +					const char *unused, void *arg)
> +{
> +	char *opt;
> +
> +	if (strcmp(param, "console"))
> +		return 0;
> +
> +	if (!val)
> +		return 0;
> +
> +	opt = strchr(val, ',');
> +	if (opt) {
> +		opt[0] = '\0';
> +		opt++;
> +	}

Can this be done without mangling val, i.e. without kstrdup below?

> +	if (!strlen(val))

IOW, can this check be "val - opt > 0" or alike?

> +		return 0;
> +
> +	return serial_base_add_con(val, opt);
> +}
> +
> +/*
> + * The "console=" option is handled by console_setup() in printk. We can't use
> + * early_param() as do_early_param() checks for "console" and "earlycon" options
> + * so console_setup() potentially handles console also early. Use parse_args().

So why not concentrate console= handling on one place, ie. in 
console_setup()? The below (second time console= handling) occurs quite 
illogical to me.

> + */
> +static int __init serial_base_opts_init(void)
> +{
> +	char *command_line;
> +
> +	command_line = kstrdup(boot_command_line, GFP_KERNEL);
> +	if (!command_line)
> +		return -ENOMEM;
> +
> +	parse_args("Setting serial core console", command_line,
> +		   NULL, 0, -1, -1, NULL, serial_base_parse_one);
> +
> +	kfree(command_line);
> +
> +	return 0;
> +}

thanks,
-- 
js
suse labs