linux-kernel - Re: [PATCH 4/7] drivers: serial: kgdboc: Check CON_SUSPENDED instead of CON

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]

Message-ID: <f962e9bab3dc8bf5cae1c7e187a54fb96a543d51.camel@suse.com>
Date: Tue, 10 Jun 2025 17:03:02 -0300
From: Marcos Paulo de Souza <mpdesouza@...e.com>
To: Doug Anderson <dianders@...omium.org>
Cc: Petr Mladek <pmladek@...e.com>, Steven Rostedt <rostedt@...dmis.org>, 
 John Ogness <john.ogness@...utronix.de>, Sergey Senozhatsky
 <senozhatsky@...omium.org>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>, Jason
 Wessel	 <jason.wessel@...driver.com>, Daniel Thompson <danielt@...nel.org>,
 Richard Weinberger <richard@....at>, Anton Ivanov
 <anton.ivanov@...bridgegreys.com>, Johannes Berg	
 <johannes@...solutions.net>, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org, kgdb-bugreport@...ts.sourceforge.net, 
	linux-um@...ts.infradead.org
Subject: Re: [PATCH 4/7] drivers: serial: kgdboc: Check CON_SUSPENDED
 instead of CON_ENABLED

On Mon, 2025-06-09 at 13:13 -0700, Doug Anderson wrote:
> Hi,
> 
> On Fri, Jun 6, 2025 at 7:54 PM Marcos Paulo de Souza
> <mpdesouza@...e.com> wrote:
> > 
> > All consoles found on for_each_console are registered, meaning that
> > all of
> > them are CON_ENABLED. The code tries to find an active console, so
> > check if the
> > console is not suspended instead.
> > 
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@...e.com>
> > ---
> >  drivers/tty/serial/kgdboc.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/tty/serial/kgdboc.c
> > b/drivers/tty/serial/kgdboc.c
> > index
> > 85f6c5a76e0fff556f86f0d45ebc5aadf5b191e8..af6d2208b8ddb82d62f33292b
> > 006b2923583a0d2 100644
> > --- a/drivers/tty/serial/kgdboc.c
> > +++ b/drivers/tty/serial/kgdboc.c
> > @@ -577,7 +577,8 @@ static int __init kgdboc_earlycon_init(char
> > *opt)
> >         console_list_lock();
> >         for_each_console(con) {
> >                 if (con->write && con->read &&
> > -                   (con->flags & (CON_BOOT | CON_ENABLED)) &&
> > +                   (con->flags & CON_BOOT) &&
> > +                   ((con->flags & CON_SUSPENDED) == 0) &&
> 
> I haven't tried running the code, so I could easily be mistaken,
> but...
> 
> ...the above doesn't seem like the correct conversion. The old
> expression was:
> 
> (con->flags & (CON_BOOT | CON_ENABLED))
> 
> That would evaluate to non-zero (true) if the console was _either_
> "boot" or "enabled".
> 
> The new expression is is:
> 
> (con->flags & CON_BOOT) && ((con->flags & CON_SUSPENDED) == 0)
> 
> That's only true if the console is _both_ "boot" and "not suspended".

My idea here was that the users of for_each_console would find the
first available console, and by available I would expect them to be
usable. In this case, is there any value for kgdboc to use a console
that is suspended? Would it work in this case?

I never really used kgdboc, but only checking if the console was
enabled (which it's always the case here) was something that needed to
be fixed.

Maybe I'm missing something here as well, so please let me know if I
should remove the new check.

> 
> -Doug