linux-kernel - Re: [PATCH v4 4/4] serial: core: Add serial_base_match_and_update_preferred

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 for Android: free password hash cracker in your pocket

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

Date: Mon, 24 Jun 2024 10:21:32 +0300
From: Tony Lindgren <tony.lindgren@...ux.intel.com>
To: Petr Mladek <pmladek@...e.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	John Ogness <john.ogness@...utronix.de>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Tony Lindgren <tony@...mide.com>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 4/4] serial: core: Add
 serial_base_match_and_update_preferred_console()

On Fri, Jun 21, 2024 at 02:37:27PM +0200, Petr Mladek wrote:
> On Thu 2024-06-20 15:45:29, Tony Lindgren wrote:
> > Let's add serial_base_match_and_update_preferred_console() for consoles
> > using DEVNAME:0.0 style naming.
> > 
> > The earlier approach to add it caused issues in the kernel command line
> > ordering as we were calling __add_preferred_console() again for the
> > deferred consoles.
> > 
> > Signed-off-by: Tony Lindgren <tony.lindgren@...ux.intel.com>
> 
> Looks good and seems to work well:
> 
> Reviewed-by: Petr Mladek <pmladek@...e.com>
> Tested-by: Petr Mladek <pmladek@...e.com>

OK thanks for testing again.

> See an idea below.
>
> > --- a/drivers/tty/serial/serial_base_bus.c
> > +++ b/drivers/tty/serial/serial_base_bus.c
...
> > +int serial_base_match_and_update_preferred_console(struct uart_driver *drv,
> > +						   struct uart_port *port)
> > +{
> > +	const char *port_match __free(kfree) = NULL;
> > +	int ret;
> > +
> > +	port_match = kasprintf(GFP_KERNEL, "%s:%d.%d", dev_name(port->dev),
> > +			       port->ctrl_id, port->port_id);
> > +	if (!port_match)
> > +		return -ENOMEM;
> 
> The name is going to be compared with:
> 
> struct console_cmdline
> {
> [...]
> 	char	devname[32];			/* DEVNAME:0.0 style device name */
> 
> It looks like an overkill to allocate such a small buffer. It would
> be perfectly fine to use a buffer on stack.
> 
> Well, we would need to define somewhere (likely in include/linux/console.h):
> 
> #define CONSOLE_DEVNAME_LEN 32
> 
> and then do
> 
> 	char port_match[CONSOLE_DEVNAME_LEN];
> 	int len;
> 
> 	len = snprintf(port_match, ARRAY_SIZE(port_match), "%s:%d.%d",
> 		       dev_name(port->dev), port->ctrl_id, port->port_id);
> 	if (len >= ARRAY_SIZE(port_match)) {
> 		pr_warn("Console devname does not fit into the buffer: "%s:%d.%d\n",
> 			 dev_name(port->dev), port->ctrl_id, port->port_id);
> 		return -ENOMEM;
> 	}
> 
> The advantage is that it would warn when there are longer device names.
> It would help to catch situations when CONSOLE_DEVNAME_LEN is not big enough.

Good idea.

> It might be done in a separate patch.

Sounds good to me.

Regards,

Tony