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: <1330403218-21045-1-git-send-email-andrey.warkentin@gmail.com>
Date:	Mon, 27 Feb 2012 23:26:58 -0500
From:	Andrei Warkentin <andrey.warkentin@...il.com>
To:	kgdb-bugreport@...ts.sourceforge.net
Cc:	linux-kernel@...r.kernel.org,
	Andrei Warkentin <andrey.warkentin@...il.com>,
	Andrei Warkentin <andreiw@...are.com>,
	Jason Wessel <jason.wessel@...driver.com>
Subject: [PATCH] KDB: Fix usability issues relating to the 'enter' key.

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.

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

Tested on QEMU. Set a bp on atkbd.c to verify no scan
code was leaked.

Cc: Andrei Warkentin <andreiw@...are.com>
Cc: Jason Wessel <jason.wessel@...driver.com>
Signed-off-by: Andrei Warkentin <andrey.warkentin@...il.com>
---
 drivers/tty/serial/kgdboc.c     |   15 ++++++
 include/linux/kdb.h             |    1 +
 kernel/debug/kdb/kdb_keyboard.c |  104 ++++++++++++++++++++++++++++++---------
 3 files changed, 98 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 2b42a01..c74b47b 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -284,6 +284,8 @@ static void kgdboc_pre_exp_handler(void)
 
 static void kgdboc_post_exp_handler(void)
 {
+	int i;
+
 	/* decrement the module count when the debugger detaches */
 	if (!kgdb_connected)
 		module_put(THIS_MODULE);
@@ -291,6 +293,19 @@ static void kgdboc_post_exp_handler(void)
 		dbg_restore_graphics = 0;
 		con_debug_leave();
 	}
+
+	/*
+	 * For i8042 keyboard, we still have the ENTER
+	 * break code in the buffer after KDB processed
+	 * the 'g' command and exited. Clean it up.
+	 */
+	for (i = 0; i < kdb_poll_idx; i++) {
+		if (kdb_poll_funcs[i] == kdb_get_kbd_char) {
+			kdb_kbd_cleanup_state();
+			break;
+		}
+	}
+
 	kgdboc_restore_input();
 }
 
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 0647258..9bf65cc 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -124,6 +124,7 @@ extern void kdb_init(int level);
 typedef int (*get_char_func)(void);
 extern get_char_func kdb_poll_funcs[];
 extern int kdb_get_kbd_char(void);
+extern void kdb_kbd_cleanup_state(void);
 
 static inline
 int kdb_process_cpu(const struct task_struct *p)
diff --git a/kernel/debug/kdb/kdb_keyboard.c b/kernel/debug/kdb/kdb_keyboard.c
index 4bca634..34a8722 100644
--- a/kernel/debug/kdb/kdb_keyboard.c
+++ b/kernel/debug/kdb/kdb_keyboard.c
@@ -25,6 +25,7 @@
 #define KBD_STAT_MOUSE_OBF	0x20	/* Mouse output buffer full */
 
 static int kbd_exists;
+static int kbd_last_ret;
 
 /*
  * Check if the keyboard controller has a keypress for us.
@@ -90,8 +91,11 @@ int kdb_get_kbd_char(void)
 		return -1;
 	}
 
-	if ((scancode & 0x80) != 0)
+	if ((scancode & 0x80) != 0) {
+		if (scancode == 0x9c)
+			kbd_last_ret = 0;
 		return -1;
+	}
 
 	scancode &= 0x7f;
 
@@ -178,35 +182,91 @@ int kdb_get_kbd_char(void)
 		return -1;	/* ignore unprintables */
 	}
 
-	if ((scancode & 0x7f) == 0x1c) {
-		/*
-		 * enter key.  All done.  Absorb the release scancode.
-		 */
+	if (scancode == 0x1c) {
+		kbd_last_ret = 1;
+		return 13;
+	}
+
+	return keychar & 0xff;
+}
+EXPORT_SYMBOL_GPL(kdb_get_kbd_char);
+
+/*
+ * Best effort cleanup of ENTER break codes
+ * on leaving KDB. Called on exiting KDB,
+ * when we know we processed an ENTER or
+ * KP ENTER scan code.
+ */
+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;
+
+	/*
+	 * Enter key. Need to absorb the break
+	 * code here, lest it gets leaked out
+	 * if we exit KDB as the result of
+	 * processing 'g'.
+	 *
+	 * This has several interesting implications:
+	 * + Need to handle KP ENTER, which has break
+	 *   code 0xe0 0x9c.
+	 * + Need to handle repeat ENTER and repeat
+	 *   KP ENTER. Repeats only get a break code
+	 *   at the end of the repeated sequence. This
+	 *   means we can't propagate the repeated key
+	 *   press, and must swallow it away.
+	 * + Need to handle possible PS/2 mouse input.
+	 * + Need to handle mashed keys.
+	 */
+
+	while (1) {
 		while ((inb(KBD_STATUS_REG) & KBD_STAT_OBF) == 0)
-			;
+			cpu_relax();
 
 		/*
-		 * Fetch the scancode
+		 * Fetch the scancode.
 		 */
 		scancode = inb(KBD_DATA_REG);
 		scanstatus = inb(KBD_STATUS_REG);
 
-		while (scanstatus & KBD_STAT_MOUSE_OBF) {
-			scancode = inb(KBD_DATA_REG);
-			scanstatus = inb(KBD_STATUS_REG);
-		}
+		/*
+		 * Skip mouse input.
+		 */
+		if (scanstatus & KBD_STAT_MOUSE_OBF)
+			continue;
 
-		if (scancode != 0x9c) {
-			/*
-			 * Wasn't an enter-release,  why not?
-			 */
-			kdb_printf("kdb: expected enter got 0x%x status 0x%x\n",
-			       scancode, scanstatus);
-		}
+		/*
+		 * If we see 0xe0, this is either
+		 * a break code for KP ENTER, or a repeat
+		 * make for KP ENTER. Either way, since
+		 * the second byte is equivalent to an ENTER,
+		 * skip the 0xe0 and try again.
+		 *
+		 * If we see 0x1c, this must be a repeat
+		 * ENTER or KP ENTER (and we swallowed 0xe0
+		 * before). Try again.
+		 *
+		 * We can also see make and break codes
+		 * for other keys mashed before or after
+		 * pressing ENTER. Thus, if we see anything
+		 * other than 0x9c, we have to try again.
+		 *
+		 * Note, if you held some key as ENTER
+		 * was depressed, that break code would
+		 * get leaked out.
+		 */
+		if (scancode != 0x9c)
+			continue;
 
-		return 13;
+		return;
 	}
-
-	return keychar & 0xff;
 }
-EXPORT_SYMBOL_GPL(kdb_get_kbd_char);
+EXPORT_SYMBOL_GPL(kdb_kbd_cleanup_state);
-- 
1.7.9.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ