linux-kernel - Re: [PATCH] KDB: Fix usability issues relating to the 'enter' key.

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: <4F4CDC50.8070505@windriver.com>
Date:	Tue, 28 Feb 2012 07:53:20 -0600
From:	Jason Wessel <jason.wessel@...driver.com>
To:	Andrei Warkentin <andrey.warkentin@...il.com>
CC:	<kgdb-bugreport@...ts.sourceforge.net>,
	<linux-kernel@...r.kernel.org>,
	Andrei Warkentin <andreiw@...are.com>
Subject: Re: [PATCH] KDB: Fix usability issues relating to the 'enter' key.

I think we are a lot closer this time.  I attached a new patch based
on your prior version.

On 02/27/2012 10:26 PM, Andrei Warkentin wrote:
> This fixes the following problems:
> 1) Typematic-repeat of 'enter' gives warning message
> and leaks make/break if KDB exits. Repeats
> look something like 0x1c 0x1c .... 0x9c
> 2) Use of 'keypad enter' gives warning message and
> leaks the ENTER break/make code out if KDB exits.
> KP ENTER repeats look someting like 0xe0 0x1c
> 0xe0 0x1c ... 0xe0 0x9c.
> 3) Lag on the order of seconds between "break" and "make" when
> expecting the enter "break" code. Seen under virtualized
> environments such as VMware ESX.
>
> The existing special enter handler tries to glob the
> enter break code, but this fails if the other (KP) enter
> was used, or if there was a key repeat. It also fails
> if you mashed some keys along with enter, and you ended
> up with a non-enter make or non-enter break code coming
> after the enter make code. So first, we modify the handler
> to handle these cases. But performing these actions on
> every enter is annoying since now you can't hold ENTER down
> to scroll <more>d messages in KDB. Since this special
> behaviour is only necessary to handle the exiting KDB
> ('g' + ENTER) without leaking scancodes to the OS, we
> perform the cleanup in kgdboc_post_exp_handler path.

This is the wrong place to do this.  The cleanup needs to get executed
any time you are going to leave kdb_main(), because there are a few
conditions like cpu switch, and transition to kgdb where you would
leak the enter code on to the buffer handler without ever returning to
the OS.  Also if you did not set CONFIG_KDB_KEYBOARD your patch did
not compile properly.  We could have fixed this by moving the code to
the input cleanup in kgdboc, but this is not needed at all if you take
a look at the revised version.

>
>
> Fixed previous regression where if kbd was not used
> to 'g' + ENTER, the cleanup code would hang.

Was this a regression in the out of tree code or something in the
mainline kdb?

> index 4bca634..34a8722 100644
> --- a/kernel/debug/kdb/kdb_keyboard.c
> +++ b/kernel/debug/kdb/kdb_keyboard.c
> +void kdb_kbd_cleanup_state(void)
> +{
> + int scancode, scanstatus;
> +
> + /*
> + * Nothing to clean up, since either
> + * ENTER was never pressed, or has already
> + * gotten cleaned up.
> + */
> + if (!kbd_last_ret)
> + return;

I added kbd_last_ret = 0; right here so this cannot get triggered a
second time if the kdb main loop is entered and exited without going
to the shell (which can happen on a soft single step operation on some archs).

Jason.

PS We can iterate other patches you sent after we settle on this one.
Thanks!

View attachment "0001-KDB-Fix-usability-issues-relating-to-the-enter-key.patch" of type "text/x-diff" (6211 bytes)