Re: [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console()

[Petr Mladek <pmladek@suse.com>]

On Tue 2026-07-14 15:39:45, John Ogness wrote:
> On 2026-06-04, Petr Mladek <pmladek@suse.com> wrote:
> > The function try_enable_preferred_console() currently has the
> > non-obvious side effect of returning success for consoles that are
> > already pre-enabled. This obscures the logic flow during console
> > registration.
> >
> > Move the check for pre-enabled consoles directly into the top-level
> > register_console(). This change makes the handling of pre-enabled
> > consoles explicit and easier to follow.
> >
> > Furthermore, this separation lays the groundwork for future cleanups
> > where try_enable_preferred_console() can be restricted to cases where
> > an entry actually exists in the console_cmdline[] array.
> >
> > Also it fixes a possible out-of-bound access when the console_cmdline[]
> > array is full and no console matched. In fact, the check of
> > c->user_specified did not make much sense. The new console either
> > matched and was handled in the for-cycle. Or it did not match
> > and then *c pointed to an unused entry.
> >
> > Possible behavior change:
> >
> > try_enable_preferred_console() will newly be called also with
> > @user_specified parameter set to "false" when it failed with the "true"
> > variant. But it looks like the right way to do. It will allow to call
> > newcon->setup() when the console was preferred by some platform
> > specific code.
> 
> I agree that it is the right way to do it. But I am a bit confused about
> the pre-enabled consoles.

You are not alone ;-)

> I understood it that a driver manually sets CON_ENABLED because that
> console is not enabled via normal methods ("console=" or "stdout" in
> device-tree). There are several examples of this (netconsole, pstore,
> nfcon, kgdb).

My understanding is that these set CON_ENABLED because they
are enabled another way, e.g. netconsole=, ramoops, ...

And console_cmdline was originally only for consoles defined
via the console= parameter, ... And it probably does not make
sense to add a fake entry into console_cmdline[] array just
to make sure that the console driver would match something.

> - drivers/tty/ehv_bytechan.c
> - drivers/tty/serial/ma35d1_serial.c
> - drivers/tty/serial/mux.c

My expectation is that these are some special platforms where the
authors just wanted them enabled out-of-box.

BTW, similar situation is also in handling SPCR on different
architectures. I have investigated it some time ago and found
the following. The console proposed by SPCR is:

  + ignored on "loongarch" architecture.

  + requested on "x86" when "acpi=spcr" is defined on the command line,
    otherwise ignored.

  + requested on "arm" architecture unless "acpi=nospcr" is defined on
    the command line.

  + requested on "riscv" architecture unless acpi is disabled

So, again, some architectures get a particular console preferred
by the particular HW by default. Others get the kernel default
which is typically a graphical ttyX.

> that are also setting CON_ENABLED, but seem that they should be
> specified in the normal way.
> 
> Are these drivers falsely setting CON_ENABLED? Is there a clear
> description about when a driver should manually set CON_ENABLED?

I think that it is intentional. And Marcos is working on some
clean up to remove the hack with CON_ENABLED completely, see
https://lore.kernel.org/all/20251227-printk-cleanup-part3-v1-0-21a291bcf197@suse.com/

> > Reported-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com> # out-of-bound access
> > Closes: https://lore.kernel.org/r/7sq4tr2nmlz32tvkf6vpsghv6exvqfghsrlvywjcqihzsqqbf7@bspclmti5xg4
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> 
> Despite my open questions about CON_ENABLED, this patch is simplifying
> the pre-enabled path and correctly specifying the possible behavior
> change.
> 
> Reviewed-by: John Ogness <john.ogness@linutronix.de>

Thanks for review.

I am going to queue this patch for 7.3. I prevents a rather
theoretical out-of-bound access to console_cmdline array,

Best Regards,
Petr