linux-kernel - Re: [PATCH v7 1/7] printk: Save console options for add_preferred_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]

Date: Tue, 28 May 2024 08:05:37 +0300
From: Tony Lindgren <tony.lindgren@...ux.intel.com>
To: Petr Mladek <pmladek@...e.com>
Cc: Tony Lindgren <tony@...mide.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>, Jonathan Corbet <corbet@....net>,
	Steven Rostedt <rostedt@...dmis.org>,
	John Ogness <john.ogness@...utronix.de>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	"David S . Miller" <davem@...emloft.net>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Dhruva Gole <d-gole@...com>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	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, Sebastian Reichel <sre@...nel.org>,
	linux-doc@...r.kernel.org
Subject: Re: [PATCH v7 1/7] printk: Save console options for
 add_preferred_console_match()

On Mon, May 27, 2024 at 03:45:55PM +0200, Petr Mladek wrote:
> On Mon 2024-05-27 14:13:19, Tony Lindgren wrote:
> > To me it seems we can fix this by keeping track of the console position
> > in the kernel command line. I'll send a fix for this to discuss.
> 
> Honestly, I would prefer some alternative solution of the whole
> problem. From my POV, the current patchset is a kind of a hack.
> 
>   1. It hides console=DEVNAME:X.Y options so that register_console()
>      does not know about them.

OK let's make register_console() aware of the DEVNAME:X.Y options.
I like what you're suggesting towards the end of your message for
this.

>   2. But wait, register_console() might then enable any random console
>      by default when there are not console= options. For this the 3rd patch
>      added @console_set_on_cmdline variable which would tell
>      register_console(): "Hey, I have hidden some user preferences.
>      I'll tell you about them when the right time comes."

That's to allow setting up a console when the driver is ready. So that 
we don't need to rely on the hardcoded device name deciphering at
console_setup() time. Maybe there's a better way to signal that though.

>   3. When port init matches the pattern, it adds the preferred console
>      so that the register_console() would know about it.
> 
>   4. But wait, the ordering of preferred consoles is important.
>      Which would require more hacks to preserve the ordering.

Preserving the ordering part is probably the smallest issue to deal with
here :) I agree we should try to make things simpler though and there
certainly are already lots of magic switches setting up the console.

>   5. Also serial_base_add_prefcon() adds the preferred console
>      with the generic name "ttyS" which is not specific
>      for the matched device. It just hopes that the very next
>      "register_console()" call will be the one related to
>      the matching device. Is this really guaranteed on SMP system?

Hmm not sure I get this issue though, when serial_base_add_prefcon() gets
called we know the device name. The "ttyS" parts are needed to avoid
relying on the hardcoded device name deciphering at console_setup() time.

If you're thinking about the serial8250_isa_init_ports() related calls,
the serial port mapping uses SERIAL_PORT_DFNS. And then a hardware
specific 8250 may take over at some point :)
 
> IMHO, the only solution would be to add a function which would
> return "ttySX" for the fiven device name.

Yes agreed, this will simplify things.

> Honestly, I do not know the hiearachy of the structures in detail.
> But the documentation in the 7th patch says:
> 
> +			The mapping of the serial ports to the tty instances
> +			can be viewed with:
> +
> +			$ ls -d /sys/bus/serial-base/devices/*:*.*/tty/*
> +			/sys/bus/serial-base/devices/00:04:0.0/tty/ttyS0
> 
> BTW: I get on my test system:
> 
> # ls -1 -d /sys/bus/serial-base/devices/*:*.*/tty/*
> /sys/bus/serial-base/devices/00:00:0.0/tty/ttyS0
> /sys/bus/serial-base/devices/serial8250:0.1/tty/ttyS1
> /sys/bus/serial-base/devices/serial8250:0.2/tty/ttyS2
> /sys/bus/serial-base/devices/serial8250:0.3/tty/ttyS3
> ...

OK

> It looks like it should be possible to provide a function which would
> return:
> 
>    "ttyS0" for "00:00:0.0"
>    "ttyS1" for "serial8250:0.1"
>    ...
> 
> 
> This function might then be used in "register_console()"
> to convert "console=DEVNAME:0.0" option to "ttyS" + "index".
> 
> The advantage would be that the relation between "DEVNAME:0.0"
> and "ttyS0" will be clear. And the code would see the same hiearachy
> as the user in /sys/bus/serial-base/devices/.

OK makes sense to me.

> Of course, I might be too naive. Maybe, the sysfs hieararchy is
> created too late. Maybe, it is not easy to go throught the
> hiearachy...
> 
> But still. I wonder if there is a straightforard way which would
> allow translation between "ttySX" and "DEVNAME:0.0" naming schemes.

We can do that on driver probe time no problem. The issues are mostly
related to setting up things early on.

Regards,

Tony