kernel/printk/printk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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
(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
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
© 2016 - 2026 Red Hat, Inc.