linux-kernel - Re: [PATCH] kdb: fix ctrl+e/a/f/b/d/p/n broken in keyboard mode

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] [day] [month] [year] [list]

Message-ID: <20241109073511.GA128495@lichtman.org>
Date: Sat, 9 Nov 2024 07:35:11 +0000
From: Nir Lichtman <nir@...htman.org>
To: Doug Anderson <dianders@...omium.org>
Cc: jason.wessel@...driver.com, daniel.thompson@...aro.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kdb: fix ctrl+e/a/f/b/d/p/n broken in keyboard mode

On Fri, Nov 08, 2024 at 04:31:12PM -0800, Doug Anderson wrote:
> Hi,
> 
> On Fri, Nov 8, 2024 at 2:11 PM Nir Lichtman <nir@...htman.org> wrote:
> >
> > Problem: When using KDB via keyboard it does not react to control
> > characters which are supported in serial mode.
> >
> > I have took them all besides the TAB control character translation (I am
> > not sure what that maps to on the keyboard)
> 
> Tab translates to 9, so the 9th character. Ctrl-I.

Noted, thanks

> 
> 
> >  kernel/debug/kdb/kdb_keyboard.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/debug/kdb/kdb_keyboard.c b/kernel/debug/kdb/kdb_keyboard.c
> > index 3c2987f46f6e..2c004abd5375 100644
> > --- a/kernel/debug/kdb/kdb_keyboard.c
> > +++ b/kernel/debug/kdb/kdb_keyboard.c
> > @@ -172,6 +172,9 @@ int kdb_get_kbd_char(void)
> >         switch (KTYP(keychar)) {
> >         case KT_LETTER:
> >         case KT_LATIN:
> > +               if (keychar == 4 || keychar == 1 || keychar == 5 || keychar == 2 ||
> > +                   keychar == 16 || keychar == 14 || keychar == 6)
> > +                       return keychar;         /* non-printable supported control characters (e.g. CTRL+A) */
> 
> This is probably OK, but IMO readability here (and above) could be
> improved. Untested, but I think:
> 
> #define CTRL(c) (c - 64)

That's a good point, I will work on a v2 with this fix.

> 
> Then:
> 
> /* Allowlist supported control characters */
> switch(keychar) {
> case CTRL('A'): /* Home */
[...]

Thanks,
Nir