Re: [PATCH] printk: fix out-of-bounds access in try_enable_preferred_console()

[Petr Mladek <pmladek@suse.com>]

On Thu 2026-06-04 12:18:27, Petr Mladek wrote:
> (Once again with corrected LKML address. I am sorry for noice.)
> 
> Adding other printk subsystem reviewers into Cc.
> 
> There is get_maintainer.pl script for this purpose. For example:
> 
> $> ./scripts/get_maintainer.pl kernel/printk/printk.c
> Petr Mladek <pmladek@suse.com> (maintainer:PRINTK)
> Steven Rostedt <rostedt@goodmis.org> (reviewer:PRINTK)
> John Ogness <john.ogness@linutronix.de> (reviewer:PRINTK)
> Sergey Senozhatsky <senozhatsky@chromium.org> (reviewer:PRINTK)
> linux-kernel@vger.kernel.org (open list)
> 
> On Sat 2026-05-30 10:18:24, Naveen Kumar Chaudhary wrote:
> > When all MAX_CMDLINECONSOLES (8) slots in console_cmdline[] are occupied
> > and none match the newly registered console, the for loop exits with
> > i == MAX_CMDLINECONSOLES and c pointing past the end of the array. The
> > subsequent access to c->user_specified is then an out-of-bounds read.
> 
> Great catch!
> 
> > This can occur when a self-enabling console (one with CON_ENABLED already
> > set), such as netconsole or pstore, calls register_console() on a system
> > where the console_cmdline[] array has been filled by a combination of
> > command-line console= parameters, ACPI SPCR, device tree stdout-path,
> > and/or arch-specific add_preferred_console() calls.
> > 
> > Add a bounds check to ensure c is only dereferenced when the loop exited
> > due to finding an empty slot (i.e., c still points within the array).
> > Also add parentheses around the bitwise-AND to silence compiler warnings
> > about its use in a boolean context.
> 
> But the fix is is not correct, see below.
> 
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -3938,7 +3938,8 @@ static int try_enable_preferred_console(struct console *newcon,
> >  	 * without matching. Accept the pre-enabled consoles only when match()
> >  	 * and setup() had a chance to be called.
> >  	 */
> > -	if (newcon->flags & CON_ENABLED && c->user_specified ==	user_specified)
> > +	if (i < MAX_CMDLINECONSOLES && (newcon->flags & CON_ENABLED) &&
> > +	    c->user_specified == user_specified)
> 
> This would prevent the out-of-bound access to c->user_specified.
> 
> But the check of c->user_specified does _not_ make sense in the first
> place.
> 
> Background:
> -----------
> 
> The idea was that we would allow to match a preferred console
> and run newcon->setup(). By other words, this code should
> be called in register_console() after
> 
> 	/* See if this console matches one we selected on the command line */
> 	err = try_enable_preferred_console(newcon, true);
> 
> 	/* If not, try to match against the platform default(s) */
> 	if (err == -ENOENT)
> 		err = try_enable_preferred_console(newcon, false);
> 
> , when try_enable_preferred_console() did not return a real error.
> The real error is an error from newcon->setup(). Note that -ENOENT
> is not meant as a real error here.
> 
> Solution:
> ---------
> 
> IMHO, the right approach is to move this code out of
> try_enable_preferred_console(). Like it is done in my clean up of
> the registration code. I have just sent v3 earlier today, see
> https://lore.kernel.org/all/20260602085312.228251-10-pmladek@suse.com/
> 
> That said, I think about backporting the 10th patch from the
> above mentioned clean up and fixing this ASAP. Better be
> safe than sorry...

JFYI, the fix for this problem is the 1st patch in v4 of the above
mentioned clean up, see
https://lore.kernel.org/all/20260604101459.393162-2-pmladek@suse.com/

It would be great if anyone could review at least this 1st patch soon.

Best Regards,
Petr