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

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

[Naveen Kumar Chaudhary]

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.

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.

Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
---
 kernel/printk/printk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0323149548f6..00282ca467fd 100644
--- 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)
 		return 0;
 
 	return -ENOENT;
-- 
2.43.0

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

[Petr Mladek]

(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/


Impact:
-------

Now, the question is how serious this bug is.
Do we need to fix it now?
Or is it enough to wait for the clean up?

IMHO, the only danger is that the kernel might trigger an invalid
access and panic(). But is this possible?

console_cmdline[] is stored in the initialized data segment. I guess
that there are always another "valid" static data right after it.
So, it should "never" trigger an invalid access.

Reading invalid data should not cause big problems.
In the worst case, the console will get enabled in
try_enable_preferred_console(newcon, true) instead
of try_enable_preferred_console(newcon, true) and
newcon->setup() won't get caller. But I think that pre-enabled
console drivers should never rely on calling this, anyway.

Finally, it is hard to imagine that anyone would use all 8 slots
for preferred console entries. So, this should be almost impossible
to hit in practice.

That said, I think about backporting the 10th patch from the
above mentioned clean up and fixing this ASAP. Better be
safe than sorry...

Best Regards,
Petr

>  		return 0;
>  
>  	return -ENOENT;
> -- 
> 2.43.0

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

[Petr Mladek]

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

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

[Steven Rostedt]

On Thu, 4 Jun 2026 12:18:24 +0200
Petr Mladek <pmladek@suse.com> wrote:

> Now, the question is how serious this bug is.
> Do we need to fix it now?
> Or is it enough to wait for the clean up?

The bug is only triggered via a specific kernel command line and/or adding
new drivers that use all the available console slots, right?

If so, then this is *not* critical and can wait for the clean up.

-- Steve