linux-kernel - Re: [PATCH] kdb: Fix incorrect naming of history arrow keys in code

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: <20241101185700.GA38961@lichtman.org>
Date: Fri, 1 Nov 2024 18:57:00 +0000
From: Nir Lichtman <nir@...htman.org>
To: Daniel Thompson <daniel.thompson@...aro.org>
Cc: Doug Anderson <dianders@...omium.org>, jason.wessel@...driver.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kdb: Fix incorrect naming of history arrow keys in code

On Fri, Nov 01, 2024 at 06:34:18PM +0000, Daniel Thompson wrote:
> On Fri, Nov 01, 2024 at 08:29:39AM -0700, Doug Anderson wrote:
> > On Thu, Oct 31, 2024 at 5:26 PM Nir Lichtman <nir@...htman.org> wrote:
> > >
> > > On Thu, Oct 31, 2024 at 04:06:03PM -0700, Doug Anderson wrote:
> > > > >
> > > > > kdb doesn't react to ctrl p and n, and following the code flow with GDB
> > > > > reveals that these values map to the up and down arrows.
> > > >
> > > > Really? kdb reacts to "ctrl-P" and "ctrl-N" for me. It also reacts to
> > > > "ctrl-F" and "ctrl-B".
> > > >
> > >
> > > Interesting, how do you run kdb? I use the kgdboc=kbd kernel boot param.
> > > I haven't checked with serial as the console since I work with the keyboard,
> > > but if serial does go through this using ctrl+p/n then the code in the
> > > current state is misleading since the keys change depending on the I/O method.
> >
> > > Evidence in the code for usage of arrow keys in the case of keyboard can
> > > be seen by examining kdb_read in kernel/debug/kdb/kdb_io.c, in the /* Down */
> > > and /* Up */ cases the values 14 and 16 can be seen.
> >
> > Right. Essentially the logic is converting the Up and Down sequences
> > to the characters Ctrl-P and Ctrl-N. ...so by the time we get to
> > handle_ctrl_cmd() the characters really are Ctrl commands, not arrow
> > commands. Thus handle_ctrl_cmd() is correct as is.
> 
> Those comments, which I'm pretty sure I added, are arguably more a
> reminder about how the input systems map up/down (which on serial are
> multi-byte escape sequences) into single character control codes.
> 
> 
> > > Do you suggest to keep as is or to work on a patch with a more generic name that
> > > would fit both?
> >
> > IMO it's a bug that the keyboard code isn't properly reporting Ctrl-N
> > and Ctrl-P. Does it handle other Ctrl characters? Ctrl-A should go to
> > the start of the line and Ctrl-E the end. Maybe you can track down why
> > this isn't happening?
> 
> I'm with Doug on this one. What values does the kdb code generate
> Ctrl-<letter>? IMHO it should be generating 14 for ctrl-N and 16 for
> ctrl-P.
> 
> BTW ctrl-N and ctrl-P are very common keystrokes for command line
> history (try them in any readline program such as bash).
> 

Hi Doug and Daniel,

In the current state using KDB in keyboard mode doesn't react to expected
CTRL chords such as CTRL+N/P/A/E, but does react to arrow keys,
Following this, I have also inspected the serial mode and over there I see
that both arrows and CTRL chords work.
>From what I understand going forward, the best solution would be to add support
for the CTRL chords in the keyboard mode as well to be in line with serial?

BTW I originally wanted to add support also for CTRL+U and CTRL+W what is your
opinion about that as a feature as well?

Thanks,
Nir