[RFC] Input: implement sysrq as an input handler

[RFC] Input: implement sysrq as an input handler

[Dmitry Torokhov]

Instead of keeping SysRq support inside of legacy keyboard driver split
it out into a separate input handler (filter). This stops most SysRq input
events from leaking into evdev clients (some events, such as first SysRq
scancode - not keycode - event, are still leaked into both legacy keyboard
and evdev).

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/char/keyboard.c |   40 +-------
 drivers/char/sysrq.c    |  222 +++++++++++++++++++++++++++++++++++++++++------
 include/linux/sysrq.h   |   23 +----
 kernel/sysctl.c         |   23 +++++
 4 files changed, 224 insertions(+), 84 deletions(-)


diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index ada25bb..50f6c01 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -38,7 +38,6 @@
 #include <linux/kbd_kern.h>
 #include <linux/kbd_diacr.h>
 #include <linux/vt_kern.h>
-#include <linux/sysrq.h>
 #include <linux/input.h>
 #include <linux/reboot.h>
 #include <linux/notifier.h>
@@ -82,8 +81,7 @@ void compute_shiftstate(void);
 typedef void (k_handler_fn)(struct vc_data *vc, unsigned char value,
 			    char up_flag);
 static k_handler_fn K_HANDLERS;
-k_handler_fn *k_handler[16] = { K_HANDLERS };
-EXPORT_SYMBOL_GPL(k_handler);
+static k_handler_fn *k_handler[16] = { K_HANDLERS };
 
 #define FN_HANDLERS\
 	fn_null,	fn_enter,	fn_show_ptregs,	fn_show_mem,\
@@ -147,22 +145,6 @@ static struct ledptr {
 	unsigned char valid:1;
 } ledptrs[3];
 
-/* Simple translation table for the SysRq keys */
-
-#ifdef CONFIG_MAGIC_SYSRQ
-unsigned char kbd_sysrq_xlate[KEY_MAX + 1] =
-        "\000\0331234567890-=\177\t"                    /* 0x00 - 0x0f */
-        "qwertyuiop[]\r\000as"                          /* 0x10 - 0x1f */
-        "dfghjkl;'`\000\\zxcv"                          /* 0x20 - 0x2f */
-        "bnm,./\000*\000 \000\201\202\203\204\205"      /* 0x30 - 0x3f */
-        "\206\207\210\211\212\000\000789-456+1"         /* 0x40 - 0x4f */
-        "230\177\000\000\213\214\000\000\000\000\000\000\000\000\000\000" /* 0x50 - 0x5f */
-        "\r\000/";                                      /* 0x60 - 0x6f */
-static int sysrq_down;
-static int sysrq_alt_use;
-#endif
-static int sysrq_alt;
-
 /*
  * Notifier list for console keyboard events
  */
@@ -1108,7 +1090,8 @@ static int emulate_raw(struct vc_data *vc, unsigned int keycode,
 			 * pressing PrtSc/SysRq alone, but simply 0x54
 			 * when pressing Alt+PrtSc/SysRq.
 			 */
-			if (sysrq_alt) {
+			if (test_bit(KEY_LEFTALT, key_down) ||
+			    test_bit(KEY_RIGHTALT, key_down)) {
 				put_queue(vc, 0x54 | up_flag);
 			} else {
 				put_queue(vc, 0xe0);
@@ -1176,8 +1159,6 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
 
 	kbd = kbd_table + vc->vc_num;
 
-	if (keycode == KEY_LEFTALT || keycode == KEY_RIGHTALT)
-		sysrq_alt = down ? keycode : 0;
 #ifdef CONFIG_SPARC
 	if (keycode == KEY_STOP)
 		sparc_l1_a_state = down;
@@ -1190,21 +1171,6 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
 			if (keycode < BTN_MISC && printk_ratelimit())
 				printk(KERN_WARNING "keyboard.c: can't emulate rawmode for keycode %d\n", keycode);
 
-#ifdef CONFIG_MAGIC_SYSRQ	       /* Handle the SysRq Hack */
-	if (keycode == KEY_SYSRQ && (sysrq_down || (down == 1 && sysrq_alt))) {
-		if (!sysrq_down) {
-			sysrq_down = down;
-			sysrq_alt_use = sysrq_alt;
-		}
-		return;
-	}
-	if (sysrq_down && !down && keycode == sysrq_alt_use)
-		sysrq_down = 0;
-	if (sysrq_down && down && !rep) {
-		handle_sysrq(kbd_sysrq_xlate[keycode], tty);
-		return;
-	}
-#endif
 #ifdef CONFIG_SPARC
 	if (keycode == KEY_A && sparc_l1_a_state) {
 		sparc_l1_a_state = 0;
diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index 1ae2de7..1883dd8 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -1,7 +1,4 @@
-/* -*- linux-c -*-
- *
- *	$Id: sysrq.c,v 1.15 1998/08/23 14:56:41 mj Exp $
- *
+/*
  *	Linux Magic System Request Key Hacks
  *
  *	(c) 1997 Martin Mares <mj@atrey.karlin.mff.cuni.cz>
@@ -10,8 +7,13 @@
  *	(c) 2000 Crutcher Dunnavant <crutcher+kernel@datastacks.com>
  *	overhauled to use key registration
  *	based upon discusions in irc://irc.openprojects.net/#kernelnewbies
+ *
+ *	Copyright (c) 2010 Dmitry Torokhov
+ *	Input handler conversion
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/sched.h>
 #include <linux/interrupt.h>
 #include <linux/mm.h>
@@ -38,33 +40,34 @@
 #include <linux/workqueue.h>
 #include <linux/hrtimer.h>
 #include <linux/oom.h>
+#include <linux/input.h>
 
 #include <asm/ptrace.h>
 #include <asm/irq_regs.h>
 
 /* Whether we react on sysrq keys or just ignore them */
-int __read_mostly __sysrq_enabled = 1;
+static int __read_mostly sysrq_enabled = 1;
+static bool __read_mostly sysrq_always_enabled;
 
-static int __read_mostly sysrq_always_enabled;
-
-int sysrq_on(void)
+static bool sysrq_on(void)
 {
-	return __sysrq_enabled || sysrq_always_enabled;
+	return sysrq_enabled || sysrq_always_enabled;
 }
 
 /*
  * A value of 1 means 'all', other nonzero values are an op mask:
  */
-static inline int sysrq_on_mask(int mask)
+static bool sysrq_on_mask(int mask)
 {
-	return sysrq_always_enabled || __sysrq_enabled == 1 ||
-						(__sysrq_enabled & mask);
+	return sysrq_always_enabled ||
+	       sysrq_enabled == 1 ||
+	       (sysrq_enabled & mask);
 }
 
 static int __init sysrq_always_enabled_setup(char *str)
 {
-	sysrq_always_enabled = 1;
-	printk(KERN_INFO "debug: sysrq always enabled.\n");
+	sysrq_always_enabled = true;
+	pr_info("sysrq always enabled.\n");
 
 	return 1;
 }
@@ -75,6 +78,7 @@ __setup("sysrq_always_enabled", sysrq_always_enabled_setup);
 static void sysrq_handle_loglevel(int key, struct tty_struct *tty)
 {
 	int i;
+
 	i = key - '0';
 	console_loglevel = 7;
 	printk("Loglevel set to %d\n", i);
@@ -100,7 +104,7 @@ static struct sysrq_key_op sysrq_SAK_op = {
 	.enable_mask	= SYSRQ_ENABLE_KEYBOARD,
 };
 #else
-#define sysrq_SAK_op (*(struct sysrq_key_op *)0)
+#define sysrq_SAK_op (*(struct sysrq_key_op *)NULL)
 #endif
 
 #ifdef CONFIG_VT
@@ -118,7 +122,7 @@ static struct sysrq_key_op sysrq_unraw_op = {
 	.enable_mask	= SYSRQ_ENABLE_KEYBOARD,
 };
 #else
-#define sysrq_unraw_op (*(struct sysrq_key_op *)0)
+#define sysrq_unraw_op (*(struct sysrq_key_op *)NULL)
 #endif /* CONFIG_VT */
 
 static void sysrq_handle_crash(int key, struct tty_struct *tty)
@@ -194,7 +198,7 @@ static struct sysrq_key_op sysrq_showlocks_op = {
 	.action_msg	= "Show Locks Held",
 };
 #else
-#define sysrq_showlocks_op (*(struct sysrq_key_op *)0)
+#define sysrq_showlocks_op (*(struct sysrq_key_op *)NULL)
 #endif
 
 #ifdef CONFIG_SMP
@@ -297,7 +301,7 @@ static struct sysrq_key_op sysrq_ftrace_dump_op = {
 	.enable_mask	= SYSRQ_ENABLE_DUMP,
 };
 #else
-#define sysrq_ftrace_dump_op (*(struct sysrq_key_op *)0)
+#define sysrq_ftrace_dump_op (*(struct sysrq_key_op *)NULL)
 #endif
 
 static void sysrq_handle_showmem(int key, struct tty_struct *tty)
@@ -476,6 +480,7 @@ struct sysrq_key_op *__sysrq_get_key_op(int key)
 	i = sysrq_key_table_key2index(key);
 	if (i != -1)
 	        op_p = sysrq_key_table[i];
+
         return op_p;
 }
 
@@ -487,11 +492,7 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
                 sysrq_key_table[i] = op_p;
 }
 
-/*
- * This is the non-locking version of handle_sysrq.  It must/can only be called
- * by sysrq key handlers, as they are inside of the lock
- */
-void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
+static void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
 {
 	struct sysrq_key_op *op_p;
 	int orig_log_level;
@@ -543,10 +544,6 @@ void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
 	spin_unlock_irqrestore(&sysrq_key_table_lock, flags);
 }
 
-/*
- * This function is called by the keyboard handler when SysRq is pressed
- * and any other keycode arrives.
- */
 void handle_sysrq(int key, struct tty_struct *tty)
 {
 	if (sysrq_on())
@@ -554,6 +551,148 @@ void handle_sysrq(int key, struct tty_struct *tty)
 }
 EXPORT_SYMBOL(handle_sysrq);
 
+/* Simple translation table for the SysRq keys */
+static const unsigned char sysrq_xlate[KEY_MAX + 1] =
+        "\000\0331234567890-=\177\t"                    /* 0x00 - 0x0f */
+        "qwertyuiop[]\r\000as"                          /* 0x10 - 0x1f */
+        "dfghjkl;'`\000\\zxcv"                          /* 0x20 - 0x2f */
+        "bnm,./\000*\000 \000\201\202\203\204\205"      /* 0x30 - 0x3f */
+        "\206\207\210\211\212\000\000789-456+1"         /* 0x40 - 0x4f */
+        "230\177\000\000\213\214\000\000\000\000\000\000\000\000\000\000" /* 0x50 - 0x5f */
+        "\r\000/";                                      /* 0x60 - 0x6f */
+
+static bool sysrq_down;
+static int sysrq_alt_use;
+static int sysrq_alt;
+
+static bool sysrq_filter(struct input_handle *handle, unsigned int type,
+		         unsigned int code, int value)
+{
+
+	if (type != EV_KEY)
+		goto out;
+
+	switch (code) {
+
+	case KEY_LEFTALT:
+	case KEY_RIGHTALT:
+		if (value)
+			sysrq_alt = code;
+		else if (sysrq_down && code == sysrq_alt_use)
+			sysrq_down = false;
+		break;
+
+	case KEY_SYSRQ:
+		if (value == 1 && sysrq_alt) {
+			sysrq_down = true;
+			sysrq_alt_use = sysrq_alt;
+		}
+
+		break;
+
+	default:
+		if (sysrq_down && value && value != 2) {
+			struct vc_data *vc = vc_cons[fg_console].d;
+
+			__handle_sysrq(sysrq_xlate[code], vc->vc_tty, 1);
+		}
+	}
+
+out:
+	return sysrq_down;
+}
+
+static int sysrq_connect(struct input_handler *handler,
+			 struct input_dev *dev,
+			 const struct input_device_id *id)
+{
+	struct input_handle *handle;
+	int error;
+
+	sysrq_down = false;
+	sysrq_alt = 0;
+
+	handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
+	if (!handle)
+		return -ENOMEM;
+
+	handle->dev = dev;
+	handle->handler = handler;
+	handle->name = "sysrq";
+
+	error = input_register_handle(handle);
+	if (error) {
+		pr_err("Failed to register input sysrq handler, error %d\n",
+			error);
+		goto err_free;
+	}
+
+	error = input_open_device(handle);
+	if (error) {
+		pr_err("Failed to open input device, error %d\n", error);
+		goto err_unregister;
+	}
+
+	return 0;
+
+ err_unregister:
+	input_unregister_handle(handle);
+ err_free:
+	kfree(handle);
+	return error;
+}
+
+static void sysrq_disconnect(struct input_handle *handle)
+{
+	input_close_device(handle);
+	input_unregister_handle(handle);
+	kfree(handle);
+}
+
+/*
+ * We are matching on KEY_LEFTALT insteard of KEY_SYSRQ because not all
+ * keyboards have SysRq ikey predefined and so user may add it to keymap
+ * later, but we expect all such keyboards to have left alt.
+ */
+static const struct input_device_id sysrq_ids[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+				INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.evbit = { BIT_MASK(EV_KEY) },
+		.keybit = { BIT_MASK(KEY_LEFTALT) },
+	},
+	{ },
+};
+
+static struct input_handler sysrq_handler = {
+	.filter		= sysrq_filter,
+	.connect	= sysrq_connect,
+	.disconnect	= sysrq_disconnect,
+	.name		= "sysrq",
+	.id_table	= sysrq_ids,
+};
+
+int sysrq_toggle_support(int enable_mask)
+{
+	bool was_enabled = sysrq_on();
+	int error;
+
+	sysrq_enabled = enable_mask;
+
+	if (was_enabled != sysrq_on()) {
+		if (sysrq_on()) {
+			error = input_register_handler(&sysrq_handler);
+			if (error)
+				pr_err("Failed to register input handler, error %d",
+					error);
+
+		} else
+			input_unregister_handler(&sysrq_handler);
+	}
+
+	return 0;
+}
+
 static int __sysrq_swap_key_ops(int key, struct sysrq_key_op *insert_op_p,
                                 struct sysrq_key_op *remove_op_p)
 {
@@ -598,6 +737,7 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
 			return -EFAULT;
 		__handle_sysrq(c, NULL, 0);
 	}
+
 	return count;
 }
 
@@ -605,10 +745,34 @@ static const struct file_operations proc_sysrq_trigger_operations = {
 	.write		= write_sysrq_trigger,
 };
 
+static void sysrq_init_procfs(void)
+{
+	if (!proc_create("sysrq-trigger", S_IWUSR, NULL,
+			 &proc_sysrq_trigger_operations))
+		pr_err("Failed to register proc interface\n");
+}
+
+#else
+
+static inline void sysrq_init_procfs(void)
+{
+}
+
+#endif
+
 static int __init sysrq_init(void)
 {
-	proc_create("sysrq-trigger", S_IWUSR, NULL, &proc_sysrq_trigger_operations);
+	int error;
+
+	sysrq_init_procfs();
+
+	if (sysrq_on()) {
+		error = input_register_handler(&sysrq_handler);
+		if (error)
+			pr_err("Failed to register input handler, error %d",
+				error);
+	}
+
 	return 0;
 }
 module_init(sysrq_init);
-#endif
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 99adcdc..5f2bdef 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -39,41 +39,30 @@ struct sysrq_key_op {
 
 #ifdef CONFIG_MAGIC_SYSRQ
 
-extern int sysrq_on(void);
-
-/*
- * Do not use this one directly:
- */
-extern int __sysrq_enabled;
-
 /* Generic SysRq interface -- you may call it from any device driver, supplying
  * ASCII code of the key, pointer to registers and kbd/tty structs (if they
  * are available -- else NULL's).
  */
 
 void handle_sysrq(int key, struct tty_struct *tty);
-void __handle_sysrq(int key, struct tty_struct *tty, int check_mask);
 int register_sysrq_key(int key, struct sysrq_key_op *op);
 int unregister_sysrq_key(int key, struct sysrq_key_op *op);
 struct sysrq_key_op *__sysrq_get_key_op(int key);
 
+int sysrq_toggle_support(int enable_mask);
+
 #else
 
-static inline int sysrq_on(void)
-{
-	return 0;
-}
-static inline int __reterr(void)
+static inline int register_sysrq_key(int key, struct sysrq_key_op *op)
 {
 	return -EINVAL;
 }
-static inline void handle_sysrq(int key, struct tty_struct *tty)
+
+static inline int unregister_sysrq_key(int key, struct sysrq_key_op *op)
 {
+	return -EINVAL;
 }
 
-#define register_sysrq_key(ig,nore) __reterr()
-#define unregister_sysrq_key(ig,nore) __reterr()
-
 #endif
 
 #endif /* _LINUX_SYSRQ_H */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 8686b0f..ce724a0 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -163,6 +163,27 @@ static int proc_taint(struct ctl_table *table, int write,
 			       void __user *buffer, size_t *lenp, loff_t *ppos);
 #endif
 
+#ifdef CONFIG_MAGIC_SYSRQ
+static int __sysrq_enabled; /* Note: sysrq code ises it's own private copy */
+
+static int sysrq_sysctl_handler(ctl_table *table, int write,
+				void __user *buffer, size_t *lenp,
+				loff_t *ppos)
+{
+	int error;
+
+	error = proc_dointvec(table, write, buffer, lenp, ppos);
+	if (error)
+		return error;
+
+	if (write)
+		sysrq_toggle_support(__sysrq_enabled);
+
+	return 0;
+}
+
+#endif
+
 static struct ctl_table root_table[];
 static struct ctl_table_root sysctl_table_root;
 static struct ctl_table_header root_table_header = {
@@ -567,7 +588,7 @@ static struct ctl_table kern_table[] = {
 		.data		= &__sysrq_enabled,
 		.maxlen		= sizeof (int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= sysrq_sysctl_handler,
 	},
 #endif
 #ifdef CONFIG_PROC_SYSCTL

-- 
Dmitry

Re: [RFC] Input: implement sysrq as an input handler

[Henrique de Moraes Holschuh]

Any chance of the user being able to avoid the SysRQ events getting to the
handle, e.g. by opening the input device in exclusive mode or something like
that?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RFC] Input: implement sysrq as an input handler

[Dmitry Torokhov]

On Thu, Mar 18, 2010 at 09:00:43PM -0300, Henrique de Moraes Holschuh wrote:
> Any chance of the user being able to avoid the SysRQ events getting to the
> handle, e.g. by opening the input device in exclusive mode or something like
> that?
>

Yes, it is a possible to suppress SysRq by grabbing an input device.
This possibility exisst with the current implementation too though -
after all legacy keyboard driver implemented as an input handler as
well.

... or am I answering a question different from the one you asked? ;)
 
-- 
Dmitry

Re: [RFC] Input: implement sysrq as an input handler

[Randy Dunlap]

On 03/16/10 23:19, Dmitry Torokhov wrote:
> Instead of keeping SysRq support inside of legacy keyboard driver split
> it out into a separate input handler (filter). This stops most SysRq input
> events from leaking into evdev clients (some events, such as first SysRq
> scancode - not keycode - event, are still leaked into both legacy keyboard
> and evdev).
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/char/keyboard.c |   40 +-------
>  drivers/char/sysrq.c    |  222 +++++++++++++++++++++++++++++++++++++++++------
>  include/linux/sysrq.h   |   23 +----
>  kernel/sysctl.c         |   23 +++++
>  4 files changed, 224 insertions(+), 84 deletions(-)


> +/* Simple translation table for the SysRq keys */
> +static const unsigned char sysrq_xlate[KEY_MAX + 1] =
> +        "\000\0331234567890-=\177\t"                    /* 0x00 - 0x0f */
> +        "qwertyuiop[]\r\000as"                          /* 0x10 - 0x1f */
> +        "dfghjkl;'`\000\\zxcv"                          /* 0x20 - 0x2f */
> +        "bnm,./\000*\000 \000\201\202\203\204\205"      /* 0x30 - 0x3f */
> +        "\206\207\210\211\212\000\000789-456+1"         /* 0x40 - 0x4f */
> +        "230\177\000\000\213\214\000\000\000\000\000\000\000\000\000\000" /* 0x50 - 0x5f */
> +        "\r\000/";                                      /* 0x60 - 0x6f */

We are running out of these precious sysrq keys.
Do you have any ideas of how to have a larger xlate table?

-- 
~Randy

Re: [RFC] Input: implement sysrq as an input handler

[Dmitry Torokhov]

On Thu, Mar 18, 2010 at 05:13:35PM -0700, Randy Dunlap wrote:
> On 03/16/10 23:19, Dmitry Torokhov wrote:
> > Instead of keeping SysRq support inside of legacy keyboard driver split
> > it out into a separate input handler (filter). This stops most SysRq input
> > events from leaking into evdev clients (some events, such as first SysRq
> > scancode - not keycode - event, are still leaked into both legacy keyboard
> > and evdev).
> > 
> > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > ---
> > 
> >  drivers/char/keyboard.c |   40 +-------
> >  drivers/char/sysrq.c    |  222 +++++++++++++++++++++++++++++++++++++++++------
> >  include/linux/sysrq.h   |   23 +----
> >  kernel/sysctl.c         |   23 +++++
> >  4 files changed, 224 insertions(+), 84 deletions(-)
> 
> 
> > +/* Simple translation table for the SysRq keys */
> > +static const unsigned char sysrq_xlate[KEY_MAX + 1] =
> > +        "\000\0331234567890-=\177\t"                    /* 0x00 - 0x0f */
> > +        "qwertyuiop[]\r\000as"                          /* 0x10 - 0x1f */
> > +        "dfghjkl;'`\000\\zxcv"                          /* 0x20 - 0x2f */
> > +        "bnm,./\000*\000 \000\201\202\203\204\205"      /* 0x30 - 0x3f */
> > +        "\206\207\210\211\212\000\000789-456+1"         /* 0x40 - 0x4f */
> > +        "230\177\000\000\213\214\000\000\000\000\000\000\000\000\000\000" /* 0x50 - 0x5f */
> > +        "\r\000/";                                      /* 0x60 - 0x6f */
> 
> We are running out of these precious sysrq keys.
> Do you have any ideas of how to have a larger xlate table?
> 

When we run out we could add shift modifiers I guess.

-- 
Dmitry

Re: [RFC] Input: implement sysrq as an input handler

[Henrique de Moraes Holschuh]

On Thu, 18 Mar 2010, Dmitry Torokhov wrote:
> On Thu, Mar 18, 2010 at 09:00:43PM -0300, Henrique de Moraes Holschuh wrote:
> > Any chance of the user being able to avoid the SysRQ events getting to the
> > handle, e.g. by opening the input device in exclusive mode or something like
> > that?
> 
> Yes, it is a possible to suppress SysRq by grabbing an input device.
> This possibility exisst with the current implementation too though -
> after all legacy keyboard driver implemented as an input handler as
> well.
> 
> ... or am I answering a question different from the one you asked? ;)

No, that's exactly what I wanted to know.

What about SAK?  That thing *has* to be untrappable.

Even for the SysRQ debug events, I'd feel better if we could have a class of
system input handlers that cannot be suppressed to use for these things.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RFC] Input: implement sysrq as an input handler

[Dmitry Torokhov]

On Fri, Mar 19, 2010 at 01:06:41PM -0300, Henrique de Moraes Holschuh wrote:
> On Thu, 18 Mar 2010, Dmitry Torokhov wrote:
> > On Thu, Mar 18, 2010 at 09:00:43PM -0300, Henrique de Moraes Holschuh wrote:
> > > Any chance of the user being able to avoid the SysRQ events getting to the
> > > handle, e.g. by opening the input device in exclusive mode or something like
> > > that?
> > 
> > Yes, it is a possible to suppress SysRq by grabbing an input device.
> > This possibility exisst with the current implementation too though -
> > after all legacy keyboard driver implemented as an input handler as
> > well.
> > 
> > ... or am I answering a question different from the one you asked? ;)
> 
> No, that's exactly what I wanted to know.
> 
> What about SAK?  That thing *has* to be untrappable.
> 

On what level untrapable? And what exactly is SAK? There is not a
special key, at least not in general case, it is an action assigned to a
key comboi.  Root can "trap" legacy keyboard SAK with loadkeys; it can
also disable sysrq, unload modules and do other nasty things. But
ordinary users can not trap it.

> Even for the SysRQ debug events, I'd feel better if we could have a class of
> system input handlers that cannot be suppressed to use for these things.

That would require moving "these things", including their state
machines, into input core otherwise it would not know what events can be
trappable and which should be passed through. Or we should get rid of
EVIOCGRAB.

Given the fact that event devices are accessible only to root I think
that current behavior is acceptable.

-- 
Dmitry

Re: [RFC] Input: implement sysrq as an input handler

[Henrique de Moraes Holschuh]

On Fri, 19 Mar 2010, Dmitry Torokhov wrote:
> On Fri, Mar 19, 2010 at 01:06:41PM -0300, Henrique de Moraes Holschuh wrote:
> > On Thu, 18 Mar 2010, Dmitry Torokhov wrote:
> > > On Thu, Mar 18, 2010 at 09:00:43PM -0300, Henrique de Moraes Holschuh wrote:
> > > > Any chance of the user being able to avoid the SysRQ events getting to the
> > > > handle, e.g. by opening the input device in exclusive mode or something like
> > > > that?
> > > 
> > > Yes, it is a possible to suppress SysRq by grabbing an input device.
> > > This possibility exisst with the current implementation too though -
> > > after all legacy keyboard driver implemented as an input handler as
> > > well.
> > > 
> > > ... or am I answering a question different from the one you asked? ;)
> > 
> > No, that's exactly what I wanted to know.
> > 
> > What about SAK?  That thing *has* to be untrappable.
> 
> On what level untrapable? And what exactly is SAK? There is not a
> special key, at least not in general case, it is an action assigned to a
> key comboi.  Root can "trap" legacy keyboard SAK with loadkeys; it can
> also disable sysrq, unload modules and do other nasty things. But
> ordinary users can not trap it.

root isn't really a problem from a security PoV (well, maybe it is if the
operation isn't constrained by capabilities).  SAK can't protect you from
root.

_Normal_ userspace behaviour running a root process is a problem if it
blocks these handles, though, both for SAK and regular SysRQ.  I have lost
count of how many times SysRQ+SUB delivered me from filesystem corruption
and very annoying problems, both at home and at work.

We are sort of trusting userspace to not break the one way out from severly
hung systems while doing its normal day-to-day operations (as opposed to
deliberately disabling SysRQ or remapping SAK, etc).

> > Even for the SysRQ debug events, I'd feel better if we could have a class of
> > system input handlers that cannot be suppressed to use for these things.
> 
> That would require moving "these things", including their state
> machines, into input core otherwise it would not know what events can be
> trappable and which should be passed through. Or we should get rid of
> EVIOCGRAB.

Maybe we can add a flags field to input devices and input handlers, to be
able to have the core behave differently when needed, without moving
everything into the input core?  Would that work, or would it need too much
churn in the core?

> Given the fact that event devices are accessible only to root I think
> that current behavior is acceptable.

I don't trust the class of programs that would want to open input devices as
root in exclusive mode.  Desktop fluff might decide to use EVIOCGRAB or open
input devices in exclusive mode for some reason, and break SysRQ.  I'd like
to preserve the hability of userspace to EVIOCGRAB if it feels there's a
need to, while preserving the kernel's hability to NEVER ignore SysRQ and
SAK while enabled.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RFC] Input: implement sysrq as an input handler

[Dmitry Torokhov]

On Fri, Mar 19, 2010 at 11:53:29PM -0300, Henrique de Moraes Holschuh wrote:
> On Fri, 19 Mar 2010, Dmitry Torokhov wrote:
> > On Fri, Mar 19, 2010 at 01:06:41PM -0300, Henrique de Moraes Holschuh wrote:
> > > On Thu, 18 Mar 2010, Dmitry Torokhov wrote:
> > > > On Thu, Mar 18, 2010 at 09:00:43PM -0300, Henrique de Moraes Holschuh wrote:
> > > > > Any chance of the user being able to avoid the SysRQ events getting to the
> > > > > handle, e.g. by opening the input device in exclusive mode or something like
> > > > > that?
> > > > 
> > > > Yes, it is a possible to suppress SysRq by grabbing an input device.
> > > > This possibility exisst with the current implementation too though -
> > > > after all legacy keyboard driver implemented as an input handler as
> > > > well.
> > > > 
> > > > ... or am I answering a question different from the one you asked? ;)
> > > 
> > > No, that's exactly what I wanted to know.
> > > 
> > > What about SAK?  That thing *has* to be untrappable.
> > 
> > On what level untrapable? And what exactly is SAK? There is not a
> > special key, at least not in general case, it is an action assigned to a
> > key comboi.  Root can "trap" legacy keyboard SAK with loadkeys; it can
> > also disable sysrq, unload modules and do other nasty things. But
> > ordinary users can not trap it.
> 
> root isn't really a problem from a security PoV (well, maybe it is if the
> operation isn't constrained by capabilities).  SAK can't protect you from
> root.
> 
> _Normal_ userspace behaviour running a root process is a problem if it
> blocks these handles, though, both for SAK and regular SysRQ.  I have lost
> count of how many times SysRQ+SUB delivered me from filesystem corruption
> and very annoying problems, both at home and at work.
> 
> We are sort of trusting userspace to not break the one way out from severly
> hung systems while doing its normal day-to-day operations (as opposed to
> deliberately disabling SysRQ or remapping SAK, etc).
> 
> > > Even for the SysRQ debug events, I'd feel better if we could have a class of
> > > system input handlers that cannot be suppressed to use for these things.
> > 
> > That would require moving "these things", including their state
> > machines, into input core otherwise it would not know what events can be
> > trappable and which should be passed through. Or we should get rid of
> > EVIOCGRAB.
> 
> Maybe we can add a flags field to input devices and input handlers, to be
> able to have the core behave differently when needed, without moving
> everything into the input core?  Would that work, or would it need too much
> churn in the core?

The problem is that device does not know what SysRq and especially SAK are.
User can reassign key codes and key symbols easily.

> 
> > Given the fact that event devices are accessible only to root I think
> > that current behavior is acceptable.
> 
> I don't trust the class of programs that would want to open input devices as
> root in exclusive mode.  Desktop fluff might decide to use EVIOCGRAB or open
> input devices in exclusive mode for some reason, and break SysRQ.  I'd like
> to preserve the hability of userspace to EVIOCGRAB if it feels there's a
> need to, while preserving the kernel's hability to NEVER ignore SysRQ and
> SAK while enabled.

I am afraid that you chose wrong verb then. You can not _preserve_ what
you do not have - legacy keyboard driver is still an input handler, and
thus can still interfere with SysRq by grabbing input devices.

I don't think we had any issues like this since 2.5 so I would not worry
about userspace too much. If anything we just need to review what stuff
we run as root (we do that anyway, right?).

-- 
Dmitry

Re: [RFC] Input: implement sysrq as an input handler

[Pavel Machek]

Hi!

> > root isn't really a problem from a security PoV (well, maybe it is if the
> > operation isn't constrained by capabilities).  SAK can't protect you from
> > root.
> > 
> > _Normal_ userspace behaviour running a root process is a problem if it
> > blocks these handles, though, both for SAK and regular SysRQ.  I have lost
> > count of how many times SysRQ+SUB delivered me from filesystem corruption
> > and very annoying problems, both at home and at work.
> > 
> > We are sort of trusting userspace to not break the one way out from severly
> > hung systems while doing its normal day-to-day operations (as opposed to
> > deliberately disabling SysRQ or remapping SAK, etc).

If userspace disables sysrq during normal operation, that makes it
useless.

If normal user could do that, that's a security problem.

> > > That would require moving "these things", including their state
> > > machines, into input core otherwise it would not know what events can be
> > > trappable and which should be passed through. Or we should get rid of
> > > EVIOCGRAB.
> > 
> > Maybe we can add a flags field to input devices and input handlers, to be
> > able to have the core behave differently when needed, without moving
> > everything into the input core?  Would that work, or would it need too much
> > churn in the core?
> 
> The problem is that device does not know what SysRq and especially SAK are.
> User can reassign key codes and key symbols easily.

That was not case in original implementation; it had hardcoded keymap.

> I don't think we had any issues like this since 2.5 so I would not worry
> about userspace too much. If anything we just need to review what stuff
> we run as root (we do that anyway, right?).

Hehe. If X can break sysrq, that's both X and sysrq problem.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] Input: implement sysrq as an input handler

[Dmitry Torokhov]

Hi Pavel,

On Thu, Apr 01, 2010 at 03:34:55PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > root isn't really a problem from a security PoV (well, maybe it is if the
> > > operation isn't constrained by capabilities).  SAK can't protect you from
> > > root.
> > > 
> > > _Normal_ userspace behaviour running a root process is a problem if it
> > > blocks these handles, though, both for SAK and regular SysRQ.  I have lost
> > > count of how many times SysRQ+SUB delivered me from filesystem corruption
> > > and very annoying problems, both at home and at work.
> > > 
> > > We are sort of trusting userspace to not break the one way out from severly
> > > hung systems while doing its normal day-to-day operations (as opposed to
> > > deliberately disabling SysRQ or remapping SAK, etc).
> 
> If userspace disables sysrq during normal operation, that makes it
> useless.
> 
> If normal user could do that, that's a security problem.
> 

Yes, and...? This patch does not change the way one enables, disables,
intercepts, etc. SysRq and SAK compared to how it was handled when SysRq
was part of keyboard _input handler_. The only thisng this patch does is
moving the code into a _separate_ input handler.

> > > > That would require moving "these things", including their state
> > > > machines, into input core otherwise it would not know what events can be
> > > > trappable and which should be passed through. Or we should get rid of
> > > > EVIOCGRAB.
> > > 
> > > Maybe we can add a flags field to input devices and input handlers, to be
> > > able to have the core behave differently when needed, without moving
> > > everything into the input core?  Would that work, or would it need too much
> > > churn in the core?
> > 
> > The problem is that device does not know what SysRq and especially SAK are.
> > User can reassign key codes and key symbols easily.
> 
> That was not case in original implementation; it had hardcoded keymap.

The earth was also flat back then and the only keyboard was AT one. SAK
was always part of keymap so could be reassinged at any time.

> 
> > I don't think we had any issues like this since 2.5 so I would not worry
> > about userspace too much. If anything we just need to review what stuff
> > we run as root (we do that anyway, right?).
> 
> Hehe. If X can break sysrq, that's both X and sysrq problem.

Root can disable Sysrq... News at 11.

-- 
Dmitry

Re: [RFC] Input: implement sysrq as an input handler

[Pavel Machek]

Hi!

> > If userspace disables sysrq during normal operation, that makes it
> > useless.
> > 
> > If normal user could do that, that's a security problem.
> > 
> 
> Yes, and...? This patch does not change the way one enables, disables,
> intercepts, etc. SysRq and SAK compared to how it was handled when SysRq
> was part of keyboard _input handler_. The only thisng this patch does is
> moving the code into a _separate_ input handler.

Yep, that was just a note.

> > > The problem is that device does not know what SysRq and especially SAK are.
> > > User can reassign key codes and key symbols easily.
> > 
> > That was not case in original implementation; it had hardcoded keymap.
> 
> The earth was also flat back then and the only keyboard was AT one. SAK
> was always part of keymap so could be reassinged at any time.

Well, there are two SAKs.

One SAK -- in keymap, is remappable and always was.

Second, sysrq-saK -- is (was?) hardcoded and not affected by
keymap. Please don't change that.

> > > I don't think we had any issues like this since 2.5 so I would not worry
> > > about userspace too much. If anything we just need to review what stuff
> > > we run as root (we do that anyway, right?).
> > 
> > Hehe. If X can break sysrq, that's both X and sysrq problem.
> 
> Root can disable Sysrq... News at 11.

Root *does* disable sysrq is indeed news ... and problem.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] Input: implement sysrq as an input handler

[Dmitry Torokhov]

On Sun, Apr 04, 2010 at 03:27:34PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > If userspace disables sysrq during normal operation, that makes it
> > > useless.
> > > 
> > > If normal user could do that, that's a security problem.
> > > 
> > 
> > Yes, and...? This patch does not change the way one enables, disables,
> > intercepts, etc. SysRq and SAK compared to how it was handled when SysRq
> > was part of keyboard _input handler_. The only thisng this patch does is
> > moving the code into a _separate_ input handler.
> 
> Yep, that was just a note.
> 
> > > > The problem is that device does not know what SysRq and especially SAK are.
> > > > User can reassign key codes and key symbols easily.
> > > 
> > > That was not case in original implementation; it had hardcoded keymap.
> > 
> > The earth was also flat back then and the only keyboard was AT one. SAK
> > was always part of keymap so could be reassinged at any time.
> 
> Well, there are two SAKs.
> 
> One SAK -- in keymap, is remappable and always was.
> 
> Second, sysrq-saK -- is (was?) hardcoded and not affected by
> keymap. Please don't change that.

Can I change Alt? Just saying... ;)

Anyway, yes, it was hardcodced in 2.4. It also was only working on AT
keyboards, and now we support much wider range.

-- 
Dmitry