[PATCH 0/6] Magic SysRq extensions

[PATCH 0/6] Magic SysRq extensions

[Andrzej Pietrasiewicz]

Some systems, e.g. chromebooks, don't have a physical SysRq key. Patch 3/6
allows configuring which key acts as SysRq. If unconfigured, the default
KEY_SYSRQ is used.

The sysrq_key_table has effectively run out of free slots. Patch 4/6
extends the said table to accommodate capital letters, so on top of
0-9 and 'a'-'z' 'A'-'Z' are added.

Userland might want to be able to signal a specifically named process
with a specific signal as a result of some SysRq action. Patch 5/6 adds
such a capability. The name of the signalled process, the name of the
signal to be delivered to it and, optionally, the expected name of the
target process parent are configured. Once configured, the action is
available under Alt-Shift-SysRq-s.

Userland might also want to be able to execute a compound action, e.g. the
famous "Raising Elephants Is So Utterly Boring", or, say, 'w' (show blocked
tasks), followed by 's' (sync), followed by 1000 ms delay and then followed
by 'c' (crash). Patch 6/6 adds such a capability. The (short) names of
component actions are specified with a string. Optional delays between
actions are specified with a colon and the amount of milliseconds, e.g.
"reis:1000ub" or "ws:1000c". Once configured, the action is available
under Alt-Shift-SysRq-c.

While at it, remove unused linux,sysrq-reset-seq handling code and the
associated binding (patches 1/6 and 2/6).

Andrzej Pietrasiewicz (6):
  tty/sysrq: Remove linux,sysrq-reset-seq
  dt-bindings: input: Remove linux,sysrq-reset-seq binding
  tty/sysrq: Allow configurable SysRq key
  tty/sysrq: Extend the sysrq_key_table to cover capital letters
  tty/sysrq: Add configurable handler to signal a process
  tty/sysrq: Add configurable handler to execute a compound action

 .../devicetree/bindings/input/input-reset.txt |  33 ---
 drivers/tty/sysrq.c                           | 268 ++++++++++++++++--
 2 files changed, 238 insertions(+), 63 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/input-reset.txt


base-commit: 2ef96a5bb12be62ef75b5828c0aab838ebb29cb8
-- 
2.17.1

[PATCH 1/6] tty/sysrq: Remove linux,sysrq-reset-seq

[Andrzej Pietrasiewicz]

Nobody in the tree uses linux,sysrq-reset-seq in Device Tree source files.
Remove the corresponding code.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/tty/sysrq.c | 37 -------------------------------------
 1 file changed, 37 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 0dc3878794fd..93202fc24308 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -720,41 +720,6 @@ static void sysrq_detect_reset_sequence(struct sysrq_state *state,
 	}
 }
 
-#ifdef CONFIG_OF
-static void sysrq_of_get_keyreset_config(void)
-{
-	u32 key;
-	struct device_node *np;
-	struct property *prop;
-	const __be32 *p;
-
-	np = of_find_node_by_path("/chosen/linux,sysrq-reset-seq");
-	if (!np) {
-		pr_debug("No sysrq node found");
-		return;
-	}
-
-	/* Reset in case a __weak definition was present */
-	sysrq_reset_seq_len = 0;
-
-	of_property_for_each_u32(np, "keyset", prop, p, key) {
-		if (key == KEY_RESERVED || key > KEY_MAX ||
-		    sysrq_reset_seq_len == SYSRQ_KEY_RESET_MAX)
-			break;
-
-		sysrq_reset_seq[sysrq_reset_seq_len++] = (unsigned short)key;
-	}
-
-	/* Get reset timeout if any. */
-	of_property_read_u32(np, "timeout-ms", &sysrq_reset_downtime_ms);
-
-	of_node_put(np);
-}
-#else
-static void sysrq_of_get_keyreset_config(void)
-{
-}
-#endif
 
 static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 {
@@ -984,8 +949,6 @@ static inline void sysrq_register_handler(void)
 {
 	int error;
 
-	sysrq_of_get_keyreset_config();
-
 	error = input_register_handler(&sysrq_handler);
 	if (error)
 		pr_err("Failed to register input handler, error %d", error);
-- 
2.17.1

Re: [PATCH 1/6] tty/sysrq: Remove linux,sysrq-reset-seq

[Dmitry Torokhov]

Hi Andrzej,

On Mon, May 11, 2020 at 03:59:13PM +0200, Andrzej Pietrasiewicz wrote:
> Nobody in the tree uses linux,sysrq-reset-seq in Device Tree source files.
> Remove the corresponding code.

Unlike platform data, we do not require or even expect that all DT users
be present in mainline, so absence if references to a feature in kernel
can not serve as justification for removal. Consider the fact that we
support DT-style binding in ACPI (which is BIOS/firmware).

That said, I am not against removing this support, but we need to make
sure that Android (where this came from) does not use this anymore.

Thanks.

-- 
Dmitry

Re: [PATCH 1/6] tty/sysrq: Remove linux,sysrq-reset-seq

[Andrzej Pietrasiewicz]

Hi Dmitry,

W dniu 11.05.2020 o 19:58, Dmitry Torokhov pisze:
> Hi Andrzej,
> 
> On Mon, May 11, 2020 at 03:59:13PM +0200, Andrzej Pietrasiewicz wrote:
>> Nobody in the tree uses linux,sysrq-reset-seq in Device Tree source files.
>> Remove the corresponding code.
> 
> Unlike platform data, we do not require or even expect that all DT users
> be present in mainline, so absence if references to a feature in kernel
> can not serve as justification for removal. Consider the fact that we
> support DT-style binding in ACPI (which is BIOS/firmware).
> 
> That said, I am not against removing this support, but we need to make
> sure that Android (where this came from) does not use this anymore.

How can I do it? Can I at all?

Andrzej

[PATCH 2/6] dt-bindings: input: Remove linux,sysrq-reset-seq binding

[Andrzej Pietrasiewicz]

Nobody in the tree uses linux,sysrq-reset-seq in Device Tree source files
and there is no code depending on it. Remove the binding document.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 .../devicetree/bindings/input/input-reset.txt | 33 -------------------
 1 file changed, 33 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/input-reset.txt

diff --git a/Documentation/devicetree/bindings/input/input-reset.txt b/Documentation/devicetree/bindings/input/input-reset.txt
deleted file mode 100644
index 1ca6cc5ebf8e..000000000000
--- a/Documentation/devicetree/bindings/input/input-reset.txt
+++ /dev/null
@@ -1,33 +0,0 @@
-Input: sysrq reset sequence
-
-A simple binding to represent a set of keys as described in
-include/uapi/linux/input.h. This is to communicate a sequence of keys to the
-sysrq driver. Upon holding the keys for a specified amount of time (if
-specified) the system is sync'ed and reset.
-
-Key sequences are global to the system but all the keys in a set must be coming
-from the same input device.
-
-The /chosen node should contain a 'linux,sysrq-reset-seq' child node to define
-a set of keys.
-
-Required property:
-keyset: array of Linux keycodes, one keycode per cell.
-
-Optional property:
-timeout-ms: duration keys must be pressed together in milliseconds before
-generating a sysrq. If omitted the system is rebooted immediately when a valid
-sequence has been recognized.
-
-Example:
-
- chosen {
-                linux,sysrq-reset-seq {
-                        keyset = <0x03
-                                  0x04
-                                  0x0a>;
-                        timeout-ms = <3000>;
-                };
-         };
-
-Would represent KEY_2, KEY_3 and KEY_9.
-- 
2.17.1

[PATCH 3/6] tty/sysrq: Allow configurable SysRq key

[Andrzej Pietrasiewicz]

There are existing machines which don't have SysRq key, e.g. chromebooks.
This patch allows configuring which key acts as SysRq. The value is passed
with sysrq's module parameter.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/tty/sysrq.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 93202fc24308..ebad9799fdc0 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
 
 #ifdef CONFIG_INPUT
 static int sysrq_reset_downtime_ms;
+static unsigned short sysrq_key = KEY_SYSRQ;
 
 /* Simple translation table for the SysRq keys */
 static const unsigned char sysrq_xlate[KEY_CNT] =
@@ -735,10 +736,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 
 		/* Simulate press and release of Alt + SysRq */
 		input_inject_event(handle, EV_KEY, alt_code, 1);
-		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
+		input_inject_event(handle, EV_KEY, sysrq_key, 1);
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
-		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
+		input_inject_event(handle, EV_KEY, sysrq_key, 0);
 		input_inject_event(handle, EV_KEY, alt_code, 0);
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
@@ -770,6 +771,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 		}
 		break;
 
+key_sysrq:
 	case KEY_SYSRQ:
 		if (value == 1 && sysrq->alt != KEY_RESERVED) {
 			sysrq->active = true;
@@ -790,11 +792,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 		 * triggering print screen function.
 		 */
 		if (sysrq->active)
-			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
+			clear_bit(sysrq_key, sysrq->handle.dev->key);
 
 		break;
 
 	default:
+		/* handle non-default sysrq key */
+		if (code == sysrq_key)
+			goto key_sysrq;
+
 		if (sysrq->active && value && value != 2) {
 			sysrq->need_reinject = false;
 			__handle_sysrq(sysrq_xlate[code], true);
@@ -995,6 +1001,8 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
 
 module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
 
+module_param(sysrq_key, ushort, 0644);
+
 #else
 
 static inline void sysrq_register_handler(void)
-- 
2.17.1

Re: [PATCH 3/6] tty/sysrq: Allow configurable SysRq key

[Greg Kroah-Hartman]

On Mon, May 11, 2020 at 03:59:15PM +0200, Andrzej Pietrasiewicz wrote:
> There are existing machines which don't have SysRq key, e.g. chromebooks.
> This patch allows configuring which key acts as SysRq. The value is passed
> with sysrq's module parameter.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/tty/sysrq.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 93202fc24308..ebad9799fdc0 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
>  
>  #ifdef CONFIG_INPUT
>  static int sysrq_reset_downtime_ms;
> +static unsigned short sysrq_key = KEY_SYSRQ;
>  
>  /* Simple translation table for the SysRq keys */
>  static const unsigned char sysrq_xlate[KEY_CNT] =
> @@ -735,10 +736,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
>  
>  		/* Simulate press and release of Alt + SysRq */
>  		input_inject_event(handle, EV_KEY, alt_code, 1);
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
> +		input_inject_event(handle, EV_KEY, sysrq_key, 1);
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>  
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
> +		input_inject_event(handle, EV_KEY, sysrq_key, 0);
>  		input_inject_event(handle, EV_KEY, alt_code, 0);
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>  
> @@ -770,6 +771,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>  		}
>  		break;
>  
> +key_sysrq:
>  	case KEY_SYSRQ:
>  		if (value == 1 && sysrq->alt != KEY_RESERVED) {
>  			sysrq->active = true;
> @@ -790,11 +792,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>  		 * triggering print screen function.
>  		 */
>  		if (sysrq->active)
> -			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
> +			clear_bit(sysrq_key, sysrq->handle.dev->key);
>  
>  		break;
>  
>  	default:
> +		/* handle non-default sysrq key */
> +		if (code == sysrq_key)
> +			goto key_sysrq;
> +
>  		if (sysrq->active && value && value != 2) {
>  			sysrq->need_reinject = false;
>  			__handle_sysrq(sysrq_xlate[code], true);
> @@ -995,6 +1001,8 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
>  
>  module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
>  
> +module_param(sysrq_key, ushort, 0644);

No documentation of this new module parameter?

:(

Re: [PATCH 3/6] tty/sysrq: Allow configurable SysRq key

[Dmitry Torokhov]

Hi Andrzej,

On Mon, May 11, 2020 at 03:59:15PM +0200, Andrzej Pietrasiewicz wrote:
> There are existing machines which don't have SysRq key, e.g. chromebooks.
> This patch allows configuring which key acts as SysRq. The value is passed
> with sysrq's module parameter.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/tty/sysrq.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 93202fc24308..ebad9799fdc0 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
>  
>  #ifdef CONFIG_INPUT
>  static int sysrq_reset_downtime_ms;
> +static unsigned short sysrq_key = KEY_SYSRQ;
>  
>  /* Simple translation table for the SysRq keys */
>  static const unsigned char sysrq_xlate[KEY_CNT] =
> @@ -735,10 +736,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
>  
>  		/* Simulate press and release of Alt + SysRq */
>  		input_inject_event(handle, EV_KEY, alt_code, 1);
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
> +		input_inject_event(handle, EV_KEY, sysrq_key, 1);
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>  
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
> +		input_inject_event(handle, EV_KEY, sysrq_key, 0);
>  		input_inject_event(handle, EV_KEY, alt_code, 0);
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);

Unfortunately this means that if I connect my external keyboard to
chromebook SysRq there will stop working, which is not great. If we want
to support this we need to figure out how to make this handling
per-device.

FWIW Chrome OS cheats and simply adds more keys to be handled as SysRq
without removing "classic" SysRq. But that hack is obviously not
suitable for the mainline.

Thanks.

-- 
Dmitry

Re: [PATCH 3/6] tty/sysrq: Allow configurable SysRq key

[Andrzej Pietrasiewicz]

Hi Dmitry,

W dniu 11.05.2020 o 20:01, Dmitry Torokhov pisze:
> Hi Andrzej,
> 
> On Mon, May 11, 2020 at 03:59:15PM +0200, Andrzej Pietrasiewicz wrote:
>> There are existing machines which don't have SysRq key, e.g. chromebooks.
>> This patch allows configuring which key acts as SysRq. The value is passed
>> with sysrq's module parameter.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>>   drivers/tty/sysrq.c | 14 +++++++++++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
>> index 93202fc24308..ebad9799fdc0 100644
>> --- a/drivers/tty/sysrq.c
>> +++ b/drivers/tty/sysrq.c
>> @@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
>>   
>>   #ifdef CONFIG_INPUT
>>   static int sysrq_reset_downtime_ms;
>> +static unsigned short sysrq_key = KEY_SYSRQ;
>>   
>>   /* Simple translation table for the SysRq keys */
>>   static const unsigned char sysrq_xlate[KEY_CNT] =
>> @@ -735,10 +736,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
>>   
>>   		/* Simulate press and release of Alt + SysRq */
>>   		input_inject_event(handle, EV_KEY, alt_code, 1);
>> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
>> +		input_inject_event(handle, EV_KEY, sysrq_key, 1);
>>   		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>>   
>> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
>> +		input_inject_event(handle, EV_KEY, sysrq_key, 0);
>>   		input_inject_event(handle, EV_KEY, alt_code, 0);
>>   		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
> 
> Unfortunately this means that if I connect my external keyboard to
> chromebook SysRq there will stop working, which is not great. If we want
> to support this we need to figure out how to make this handling
> per-device.

I see your point.

So what you envision is SysRq key being configured for each input device
separately. I can see these problems:

- How to attach such a configuration information to each specific
device instance? It is easy if done framework-wise, but gets messy
if done per-device. And we are talking per-device rather than per-driver.

- If a user has multiple USB keyboards connected (possibly through a cascade
of hubs), how would they know which key is valid for which keyboard?

Wouldn't it be better if such a piece of configuration were valid for the
whole system instead of per-device?

Andrzej

[PATCH] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

There exist machines which don't have SysRq key at all, e.g. chromebooks.

This patch allows configuring an alternative key to act as SysRq. Devices
which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
but other devices use the alternative SysRq key instead, by default F10.
Which key is actually used can be modified with sysrq's module parameter.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 0dc3878794fd..e1d271c84746 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
 
 #ifdef CONFIG_INPUT
 static int sysrq_reset_downtime_ms;
+static unsigned short alternative_sysrq_key = KEY_F10;
 
 /* Simple translation table for the SysRq keys */
 static const unsigned char sysrq_xlate[KEY_CNT] =
@@ -621,6 +622,7 @@ struct sysrq_state {
 	unsigned long key_down[BITS_TO_LONGS(KEY_CNT)];
 	unsigned int alt;
 	unsigned int alt_use;
+	unsigned short sysrq_key;
 	bool active;
 	bool need_reinject;
 	bool reinjecting;
@@ -770,10 +772,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 
 		/* Simulate press and release of Alt + SysRq */
 		input_inject_event(handle, EV_KEY, alt_code, 1);
-		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
+		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 1);
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
-		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
+		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 0);
 		input_inject_event(handle, EV_KEY, alt_code, 0);
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
@@ -805,6 +807,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 		}
 		break;
 
+key_sysrq:
 	case KEY_SYSRQ:
 		if (value == 1 && sysrq->alt != KEY_RESERVED) {
 			sysrq->active = true;
@@ -825,11 +828,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 		 * triggering print screen function.
 		 */
 		if (sysrq->active)
-			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
+			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
 
 		break;
 
 	default:
+		/* handle non-default sysrq key */
+		if (code == sysrq->sysrq_key)
+			goto key_sysrq;
+
 		if (sysrq->active && value && value != 2) {
 			sysrq->need_reinject = false;
 			__handle_sysrq(sysrq_xlate[code], true);
@@ -924,6 +931,14 @@ static int sysrq_connect(struct input_handler *handler,
 	sysrq->handle.private = sysrq;
 	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
 
+	if (test_bit(KEY_SYSRQ, dev->keybit)) {
+		sysrq->sysrq_key = KEY_SYSRQ;
+		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
+	} else {
+		sysrq->sysrq_key = alternative_sysrq_key;
+		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);
+	}
+
 	error = input_register_handle(&sysrq->handle);
 	if (error) {
 		pr_err("Failed to register input sysrq handler, error %d\n",
@@ -1032,6 +1047,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
 
 module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
 
+module_param(alternative_sysrq_key, ushort, 0644);
+MODULE_PARM_DESC(alternative_sysrq_key,
+	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"
+	"Example\n"
+	"Using F9 as SysRq:\n"
+	"sysrq.alternative_sysrq_key=0x43\n");
+
 #else
 
 static inline void sysrq_register_handler(void)

base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
-- 
2.17.1

Re: [PATCH] tty/sysrq: Add alternative SysRq key

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

Hi!

> There exist machines which don't have SysRq key at all, e.g. chromebooks.
> 
> This patch allows configuring an alternative key to act as SysRq. Devices
> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
> but other devices use the alternative SysRq key instead, by default F10.
> Which key is actually used can be modified with sysrq's module parameter.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

So... SysRq was selected because you are not going to press
Alt-Printscreen-X by default.

I'm not sure if F10 is similar level of "impossible to press by
mistake", altrough holding up F10-B is likely not too common. Maybe it
should be some combination for chromebooks?
Leftshift-rightshift-F10-key? Ctrl-alt-del-key? :-).

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Hi Pavel,

W dniu 21.06.2020 o 23:21, Pavel Machek pisze:
> Hi!
> 
>> There exist machines which don't have SysRq key at all, e.g. chromebooks.
>>
>> This patch allows configuring an alternative key to act as SysRq. Devices
>> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
>> but other devices use the alternative SysRq key instead, by default F10.
>> Which key is actually used can be modified with sysrq's module parameter.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> 
> So... SysRq was selected because you are not going to press
> Alt-Printscreen-X by default.

This patch does not change the Alt-PrintScreen/SysRq-something sequence.
What it does instead is making the 'PrintScreen/SysRq' component of the
sequence configurable for input devices which don't declare KEY_SYSRQ in
their 'keybit' bitmap, so the sequence becomes:

Alt-<alternative sysrq key>-something

If the alternative sysrq key is used (i.e. the input device in question
does not declare KEY_SYSRQ), it is F10 by default and _that_ can be changed
with the module parameter.

To summarize:

- devices which do declare KEY_SYSRQ must use Alt-PrintScreen/SysRq-something
- devices which don't declare KEY_SYSRQ must use Alt-F10-something, but F10
can be changed with a module parameter to something else

Regards,

Andrzej

Re: [PATCH] tty/sysrq: Add alternative SysRq key

[Jiri Slaby]

On 19. 06. 20, 18:28, Andrzej Pietrasiewicz wrote:
> There exist machines which don't have SysRq key at all, e.g. chromebooks.
> 
> This patch allows configuring an alternative key to act as SysRq. Devices
> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
> but other devices use the alternative SysRq key instead, by default F10.
> Which key is actually used can be modified with sysrq's module parameter.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 0dc3878794fd..e1d271c84746 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
>  
>  #ifdef CONFIG_INPUT
>  static int sysrq_reset_downtime_ms;
> +static unsigned short alternative_sysrq_key = KEY_F10;

I would go for sysrq_alternative_key to preserve the namespace naming.

> @@ -825,11 +828,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>  		 * triggering print screen function.
>  		 */
>  		if (sysrq->active)
> -			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
> +			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
>  
>  		break;
>  
>  	default:
> +		/* handle non-default sysrq key */
> +		if (code == sysrq->sysrq_key)
> +			goto key_sysrq;
> +
>  		if (sysrq->active && value && value != 2) {
>  			sysrq->need_reinject = false;
>  			__handle_sysrq(sysrq_xlate[code], true);
> @@ -924,6 +931,14 @@ static int sysrq_connect(struct input_handler *handler,
>  	sysrq->handle.private = sysrq;
>  	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
>  
> +	if (test_bit(KEY_SYSRQ, dev->keybit)) {
> +		sysrq->sysrq_key = KEY_SYSRQ;
> +		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
> +	} else {
> +		sysrq->sysrq_key = alternative_sysrq_key;
> +		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);

Capital U, lowercase u above. Do we want to print the info in the
default case, actually?

> +	}
> +
>  	error = input_register_handle(&sysrq->handle);
>  	if (error) {
>  		pr_err("Failed to register input sysrq handler, error %d\n",
> @@ -1032,6 +1047,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
>  
>  module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
>  
> +module_param(alternative_sysrq_key, ushort, 0644);
> +MODULE_PARM_DESC(alternative_sysrq_key,
> +	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"

If you line-break here, I suggest adding a \t or two to the beginning of
the following lines:

> +	"Example\n"
> +	"Using F9 as SysRq:\n"
> +	"sysrq.alternative_sysrq_key=0x43\n");

The last \n is superfluous, there would be an empty line.

Looking at emulate_raw in drivers/tty/vt/keyboard.c, you seem you need
to update that one as well. Otherwise raw keyboard mode won't send sysrq
sequence, when you press it, right?

thanks,
-- 
js
suse labs

Re: [PATCH] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Hi Jiri,

W dniu 22.06.2020 o 08:24, Jiri Slaby pisze:
> On 19. 06. 20, 18:28, Andrzej Pietrasiewicz wrote:
>> There exist machines which don't have SysRq key at all, e.g. chromebooks.
>>
>> This patch allows configuring an alternative key to act as SysRq. Devices
>> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
>> but other devices use the alternative SysRq key instead, by default F10.
>> Which key is actually used can be modified with sysrq's module parameter.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>>   drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
>>   1 file changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
>> index 0dc3878794fd..e1d271c84746 100644
>> --- a/drivers/tty/sysrq.c
>> +++ b/drivers/tty/sysrq.c
>> @@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
>>   
>>   #ifdef CONFIG_INPUT
>>   static int sysrq_reset_downtime_ms;
>> +static unsigned short alternative_sysrq_key = KEY_F10;
> 
> I would go for sysrq_alternative_key to preserve the namespace naming.
> 
>> @@ -825,11 +828,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>>   		 * triggering print screen function.
>>   		 */
>>   		if (sysrq->active)
>> -			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
>> +			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
>>   
>>   		break;
>>   
>>   	default:
>> +		/* handle non-default sysrq key */
>> +		if (code == sysrq->sysrq_key)
>> +			goto key_sysrq;
>> +
>>   		if (sysrq->active && value && value != 2) {
>>   			sysrq->need_reinject = false;
>>   			__handle_sysrq(sysrq_xlate[code], true);
>> @@ -924,6 +931,14 @@ static int sysrq_connect(struct input_handler *handler,
>>   	sysrq->handle.private = sysrq;
>>   	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
>>   
>> +	if (test_bit(KEY_SYSRQ, dev->keybit)) {
>> +		sysrq->sysrq_key = KEY_SYSRQ;
>> +		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
>> +	} else {
>> +		sysrq->sysrq_key = alternative_sysrq_key;
>> +		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);
> 
> Capital U, lowercase u above. Do we want to print the info in the
> default case, actually?
> 
>> +	}
>> +
>>   	error = input_register_handle(&sysrq->handle);
>>   	if (error) {
>>   		pr_err("Failed to register input sysrq handler, error %d\n",
>> @@ -1032,6 +1047,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
>>   
>>   module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
>>   
>> +module_param(alternative_sysrq_key, ushort, 0644);
>> +MODULE_PARM_DESC(alternative_sysrq_key,
>> +	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"
> 
> If you line-break here, I suggest adding a \t or two to the beginning of
> the following lines:
> 
>> +	"Example\n"
>> +	"Using F9 as SysRq:\n"
>> +	"sysrq.alternative_sysrq_key=0x43\n");
> 
> The last \n is superfluous, there would be an empty line.
> 
> Looking at emulate_raw in drivers/tty/vt/keyboard.c, you seem you need
> to update that one as well. Otherwise raw keyboard mode won't send sysrq
> sequence, when you press it, right?
> 

The way I understand the input subsystem kbd_handler and sysrq_handler are
independent. The purpose of the former is to provide keystrokes to the
(current) terminal, while the purpose of the latter is to execute predefined
actions when specific key combinations are pressed. What is more,
the sysrq_handler is actually a filter and when one of its actions is triggered
then the corresponding keystrokes are filtered-out.

Regards,

Andrzej

Re: [PATCH] tty/sysrq: Add alternative SysRq key

[Dmitry Torokhov]

Hi Andrzej,

On Fri, Jun 19, 2020 at 06:28:19PM +0200, Andrzej Pietrasiewicz wrote:
> There exist machines which don't have SysRq key at all, e.g. chromebooks.
> 
> This patch allows configuring an alternative key to act as SysRq. Devices
> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
> but other devices use the alternative SysRq key instead, by default F10.
> Which key is actually used can be modified with sysrq's module parameter.

I guess you will be removing KEY_SYSRQ form all Chrome OS internal AT
keyboards and external USB keyboard with Chrome OS layouts as well? Via
udev keymap? I suppose this could work... Or have a per device setting
as we allocate a separate handle for each input device attached to the
SysRq handler.

> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 0dc3878794fd..e1d271c84746 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
>  
>  #ifdef CONFIG_INPUT
>  static int sysrq_reset_downtime_ms;
> +static unsigned short alternative_sysrq_key = KEY_F10;
>  
>  /* Simple translation table for the SysRq keys */
>  static const unsigned char sysrq_xlate[KEY_CNT] =
> @@ -621,6 +622,7 @@ struct sysrq_state {
>  	unsigned long key_down[BITS_TO_LONGS(KEY_CNT)];
>  	unsigned int alt;
>  	unsigned int alt_use;
> +	unsigned short sysrq_key;
>  	bool active;
>  	bool need_reinject;
>  	bool reinjecting;
> @@ -770,10 +772,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
>  
>  		/* Simulate press and release of Alt + SysRq */
>  		input_inject_event(handle, EV_KEY, alt_code, 1);
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
> +		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 1);
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>  
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
> +		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 0);
>  		input_inject_event(handle, EV_KEY, alt_code, 0);
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>  
> @@ -805,6 +807,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>  		}
>  		break;
>  
> +key_sysrq:
>  	case KEY_SYSRQ:
>  		if (value == 1 && sysrq->alt != KEY_RESERVED) {
>  			sysrq->active = true;
> @@ -825,11 +828,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>  		 * triggering print screen function.
>  		 */
>  		if (sysrq->active)
> -			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
> +			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
>  
>  		break;
>  
>  	default:
> +		/* handle non-default sysrq key */
> +		if (code == sysrq->sysrq_key)
> +			goto key_sysrq;
> +
>  		if (sysrq->active && value && value != 2) {
>  			sysrq->need_reinject = false;
>  			__handle_sysrq(sysrq_xlate[code], true);
> @@ -924,6 +931,14 @@ static int sysrq_connect(struct input_handler *handler,
>  	sysrq->handle.private = sysrq;
>  	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
>  
> +	if (test_bit(KEY_SYSRQ, dev->keybit)) {
> +		sysrq->sysrq_key = KEY_SYSRQ;
> +		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
> +	} else {
> +		sysrq->sysrq_key = alternative_sysrq_key;
> +		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);
> +	}

This is way too noisy IMO.

> +
>  	error = input_register_handle(&sysrq->handle);
>  	if (error) {
>  		pr_err("Failed to register input sysrq handler, error %d\n",
> @@ -1032,6 +1047,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
>  
>  module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
>  
> +module_param(alternative_sysrq_key, ushort, 0644);
> +MODULE_PARM_DESC(alternative_sysrq_key,
> +	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"
> +	"Example\n"
> +	"Using F9 as SysRq:\n"
> +	"sysrq.alternative_sysrq_key=0x43\n");
> +
>  #else
>  
>  static inline void sysrq_register_handler(void)
> 
> base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
> -- 
> 2.17.1
> 

-- 
Dmitry

Re: [PATCH] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Hi Dmitry,

W dniu 09.07.2020 o 07:05, Dmitry Torokhov pisze:
> Hi Andrzej,
> 
> On Fri, Jun 19, 2020 at 06:28:19PM +0200, Andrzej Pietrasiewicz wrote:
>> There exist machines which don't have SysRq key at all, e.g. chromebooks.
>>
>> This patch allows configuring an alternative key to act as SysRq. Devices
>> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
>> but other devices use the alternative SysRq key instead, by default F10.
>> Which key is actually used can be modified with sysrq's module parameter.
> 
> I guess you will be removing KEY_SYSRQ form all Chrome OS internal AT
> keyboards and external USB keyboard with Chrome OS layouts as well? Via
> udev keymap? I suppose this could work... Or have a per device setting
> as we allocate a separate handle for each input device attached to the
> SysRq handler.
> 

To me it makes most sense to have the decision taken per each input
device - if it is capable of providing KEY_SYSRQ, then it is used,
otherwise the alternative is taken.

The question is how to provide this information at ->connect() time.

Ideally chromebook's keyboard should be modelled in such a way that
it reflects reality. And the reality is that chromebooks probably
declare they use full AT PS/2 keyboard even though they have less keys.

It is unclear to me whether it makes sense to struggle to better
reflect actual keys repertoire at the kernel level. If udev's keymap
can be used, that should do. Now, are we able to guarantee that the
modification of the keyboard layout happens before the sysrq handler
is matched against the keyboard?

Andrzej

>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>>   drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
>>   1 file changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
>> index 0dc3878794fd..e1d271c84746 100644
>> --- a/drivers/tty/sysrq.c
>> +++ b/drivers/tty/sysrq.c
>> @@ -604,6 +604,7 @@ EXPORT_SYMBOL(handle_sysrq);
>>   
>>   #ifdef CONFIG_INPUT
>>   static int sysrq_reset_downtime_ms;
>> +static unsigned short alternative_sysrq_key = KEY_F10;
>>   
>>   /* Simple translation table for the SysRq keys */
>>   static const unsigned char sysrq_xlate[KEY_CNT] =
>> @@ -621,6 +622,7 @@ struct sysrq_state {
>>   	unsigned long key_down[BITS_TO_LONGS(KEY_CNT)];
>>   	unsigned int alt;
>>   	unsigned int alt_use;
>> +	unsigned short sysrq_key;
>>   	bool active;
>>   	bool need_reinject;
>>   	bool reinjecting;
>> @@ -770,10 +772,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
>>   
>>   		/* Simulate press and release of Alt + SysRq */
>>   		input_inject_event(handle, EV_KEY, alt_code, 1);
>> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
>> +		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 1);
>>   		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>>   
>> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
>> +		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 0);
>>   		input_inject_event(handle, EV_KEY, alt_code, 0);
>>   		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>>   
>> @@ -805,6 +807,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>>   		}
>>   		break;
>>   
>> +key_sysrq:
>>   	case KEY_SYSRQ:
>>   		if (value == 1 && sysrq->alt != KEY_RESERVED) {
>>   			sysrq->active = true;
>> @@ -825,11 +828,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>>   		 * triggering print screen function.
>>   		 */
>>   		if (sysrq->active)
>> -			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
>> +			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
>>   
>>   		break;
>>   
>>   	default:
>> +		/* handle non-default sysrq key */
>> +		if (code == sysrq->sysrq_key)
>> +			goto key_sysrq;
>> +
>>   		if (sysrq->active && value && value != 2) {
>>   			sysrq->need_reinject = false;
>>   			__handle_sysrq(sysrq_xlate[code], true);
>> @@ -924,6 +931,14 @@ static int sysrq_connect(struct input_handler *handler,
>>   	sysrq->handle.private = sysrq;
>>   	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
>>   
>> +	if (test_bit(KEY_SYSRQ, dev->keybit)) {
>> +		sysrq->sysrq_key = KEY_SYSRQ;
>> +		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
>> +	} else {
>> +		sysrq->sysrq_key = alternative_sysrq_key;
>> +		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);
>> +	}
> 
> This is way too noisy IMO.
> 
>> +
>>   	error = input_register_handle(&sysrq->handle);
>>   	if (error) {
>>   		pr_err("Failed to register input sysrq handler, error %d\n",
>> @@ -1032,6 +1047,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
>>   
>>   module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
>>   
>> +module_param(alternative_sysrq_key, ushort, 0644);
>> +MODULE_PARM_DESC(alternative_sysrq_key,
>> +	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"
>> +	"Example\n"
>> +	"Using F9 as SysRq:\n"
>> +	"sysrq.alternative_sysrq_key=0x43\n");
>> +
>>   #else
>>   
>>   static inline void sysrq_register_handler(void)
>>
>> base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
>> -- 
>> 2.17.1
>>
> 

[PATCH 4/6] tty/sysrq: Extend the sysrq_key_table to cover capital letters

[Andrzej Pietrasiewicz]

All slots in sysrq_key_table[] are either used, reserved or at least
commented with their intended use. This patch adds capital letter versions
available, which means adding 26 more entries.

For already existing SysRq operations the user presses Alt-SysRq-<key>, and
for the newly added ones Alt-Shift-SysRq-<key>.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/tty/sysrq.c | 49 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index ebad9799fdc0..ab4121a446b4 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -440,7 +440,7 @@ static struct sysrq_key_op sysrq_unrt_op = {
 /* Key Operations table and lock */
 static DEFINE_SPINLOCK(sysrq_key_table_lock);
 
-static struct sysrq_key_op *sysrq_key_table[36] = {
+static struct sysrq_key_op *sysrq_key_table[62] = {
 	&sysrq_loglevel_op,		/* 0 */
 	&sysrq_loglevel_op,		/* 1 */
 	&sysrq_loglevel_op,		/* 2 */
@@ -497,6 +497,32 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
 	/* y: May be registered on sparc64 for global register dump */
 	NULL,				/* y */
 	&sysrq_ftrace_dump_op,		/* z */
+	NULL,				/* A */
+	NULL,				/* B */
+	NULL,				/* C */
+	NULL,				/* D */
+	NULL,				/* E */
+	NULL,				/* F */
+	NULL,				/* G */
+	NULL,				/* H */
+	NULL,				/* I */
+	NULL,				/* J */
+	NULL,				/* K */
+	NULL,				/* L */
+	NULL,				/* M */
+	NULL,				/* N */
+	NULL,				/* O */
+	NULL,				/* P */
+	NULL,				/* Q */
+	NULL,				/* R */
+	NULL,				/* S */
+	NULL,				/* T */
+	NULL,				/* U */
+	NULL,				/* V */
+	NULL,				/* W */
+	NULL,				/* X */
+	NULL,				/* Y */
+	NULL,				/* Z */
 };
 
 /* key2index calculation, -1 on invalid index */
@@ -508,6 +534,8 @@ static int sysrq_key_table_key2index(int key)
 		retval = key - '0';
 	else if ((key >= 'a') && (key <= 'z'))
 		retval = key + 10 - 'a';
+	else if ((key >= 'A') && (key <= 'Z'))
+		retval = key + 36 - 'A';
 	else
 		retval = -1;
 	return retval;
@@ -622,6 +650,8 @@ struct sysrq_state {
 	unsigned long key_down[BITS_TO_LONGS(KEY_CNT)];
 	unsigned int alt;
 	unsigned int alt_use;
+	unsigned int shift;
+	unsigned int shift_use;
 	bool active;
 	bool need_reinject;
 	bool reinjecting;
@@ -771,11 +801,21 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 		}
 		break;
 
+	case KEY_LEFTSHIFT:
+	case KEY_RIGHTSHIFT:
+		if (!value)
+			sysrq->shift = KEY_RESERVED;
+		else if (value != 2)
+			sysrq->shift = code;
+		break;
+
 key_sysrq:
 	case KEY_SYSRQ:
 		if (value == 1 && sysrq->alt != KEY_RESERVED) {
 			sysrq->active = true;
 			sysrq->alt_use = sysrq->alt;
+			/* either RESERVED (for released) or actual code */
+			sysrq->shift_use = sysrq->shift;
 			/*
 			 * If nothing else will be pressed we'll need
 			 * to re-inject Alt-SysRq keysroke.
@@ -802,8 +842,13 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 			goto key_sysrq;
 
 		if (sysrq->active && value && value != 2) {
+			unsigned char c = sysrq_xlate[code];
+
 			sysrq->need_reinject = false;
-			__handle_sysrq(sysrq_xlate[code], true);
+			if (sysrq->shift_use != KEY_RESERVED)
+				if (c >= 'a' && c <= 'z')
+					c &= ~(1 << 5); /* to uppercase */
+			__handle_sysrq(c, true);
 		}
 		break;
 	}
-- 
2.17.1

[PATCH 5/6] tty/sysrq: Add configurable handler to signal a process

[Andrzej Pietrasiewicz]

Some userland might want to implement a policy to signal a configured
process with a configured signal. This patch adds necessary kernel-side
infrastructure and the newly added handler is triggered with
Alt-Shift-SysRq-s. Optionally the userland can also specify the expected
name of parent process of the victim.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/tty/sysrq.c | 123 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index ab4121a446b4..a6e91e4ae304 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -437,6 +437,15 @@ static struct sysrq_key_op sysrq_unrt_op = {
 	.enable_mask	= SYSRQ_ENABLE_RTNICE,
 };
 
+static void sysrq_signal_configured(int key);
+
+static struct sysrq_key_op sysrq_signal_configured_op = {
+	.handler	= sysrq_signal_configured,
+	.help_msg	= "signal-configured-process(S)",
+	.action_msg	= "Signal configured process",
+	.enable_mask	= SYSRQ_ENABLE_SIGNAL,
+};
+
 /* Key Operations table and lock */
 static DEFINE_SPINLOCK(sysrq_key_table_lock);
 
@@ -515,7 +524,7 @@ static struct sysrq_key_op *sysrq_key_table[62] = {
 	NULL,				/* P */
 	NULL,				/* Q */
 	NULL,				/* R */
-	NULL,				/* S */
+	&sysrq_signal_configured_op,	/* S */
 	NULL,				/* T */
 	NULL,				/* U */
 	NULL,				/* V */
@@ -633,6 +642,10 @@ EXPORT_SYMBOL(handle_sysrq);
 #ifdef CONFIG_INPUT
 static int sysrq_reset_downtime_ms;
 static unsigned short sysrq_key = KEY_SYSRQ;
+static char *sysrq_signalled;
+static char *sysrq_signalled_parent;
+static char *sysrq_signal;
+static int sysrq_signal_code;
 
 /* Simple translation table for the SysRq keys */
 static const unsigned char sysrq_xlate[KEY_CNT] =
@@ -751,6 +764,106 @@ static void sysrq_detect_reset_sequence(struct sysrq_state *state,
 	}
 }
 
+struct signal_search {
+	const char *name;
+	int code;
+};
+
+static void sysrq_str_to_signal(void)
+{
+	static const struct signal_search signals[] = {
+		{"SIGHUP", SIGHUP},
+		{"SIGINT", SIGINT},
+		{"SIGQUIT", SIGQUIT},
+		{"SIGILL", SIGILL},
+		{"SIGTRAP", SIGTRAP},
+		{"SIGABRT", SIGABRT},
+		{"SIGIOT", SIGIOT},
+		{"SIGBUS", SIGBUS},
+		{"SIGFPE", SIGFPE},
+		{"SIGKILL", SIGKILL},
+		{"SIGUSR1", SIGUSR1},
+		{"SIGSEGV", SIGSEGV},
+		{"SIGUSR2", SIGUSR2},
+		{"SIGPIPE", SIGPIPE},
+		{"SIGALRM", SIGALRM},
+		{"SIGTERM", SIGTERM},
+		{"SIGSTKFLT", SIGSTKFLT},
+		{"SIGCHLD", SIGCHLD},
+		{"SIGCONT", SIGCONT},
+		{"SIGSTOP", SIGSTOP},
+		{"SIGTSTP", SIGTSTP},
+		{"SIGTTIN", SIGTTIN},
+		{"SIGTTOU", SIGTTOU},
+		{"SIGURG", SIGURG},
+		{"SIGXCPU", SIGXCPU},
+		{"SIGXFSZ", SIGXFSZ},
+		{"SIGVTALRM", SIGVTALRM},
+		{"SIGPROF", SIGPROF},
+		{"SIGWINCH", SIGWINCH},
+		{"SIGIO", SIGIO},
+		{"SIGPOLL", SIGPOLL},
+		{"SIGPWR", SIGPWR},
+		{"SIGSYS", SIGSYS},
+	};
+	int i;
+
+	if (!sysrq_signal)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(signals); ++i)
+		if (!strcmp(signals[i].name, sysrq_signal))
+			break;
+
+	if (i >= ARRAY_SIZE(signals)) {
+		pr_err("Unknown signal name %s", sysrq_signal);
+
+		return;
+	}
+
+	sysrq_signal_code = signals[i].code;
+}
+
+static void sysrq_signal_configured(int key)
+{
+	struct task_struct *p;
+
+	sysrq_str_to_signal();
+
+	if (!sysrq_signalled) {
+		pr_err("Unconfigured process name for %s",
+		       sysrq_signal_configured_op.help_msg);
+
+		return;
+	}
+
+	if (!sysrq_signal_code) {
+		pr_err("Unconfigured signal for %s",
+		       sysrq_signal_configured_op.help_msg);
+
+		return;
+	}
+
+	read_lock(&tasklist_lock);
+	for_each_process(p) {
+		if (p->flags & (PF_KTHREAD | PF_EXITING))
+			continue;
+		if (is_global_init(p))
+			continue;
+		if (strncmp(p->comm, sysrq_signalled, TASK_COMM_LEN))
+			continue;
+		if (sysrq_signalled_parent
+		    && strncmp(p->parent->comm, sysrq_signalled_parent,
+			       TASK_COMM_LEN))
+			continue;
+
+		pr_err("%s: signal %d %s pid %u tgid %u\n", __func__,
+		       sysrq_signal_code, sysrq_signalled, p->pid, p->tgid);
+		do_send_sig_info(sysrq_signal_code, SEND_SIG_PRIV, p, true);
+	}
+	read_unlock(&tasklist_lock);
+}
+
 
 static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 {
@@ -1048,8 +1161,16 @@ module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
 
 module_param(sysrq_key, ushort, 0644);
 
+module_param(sysrq_signalled, charp, 0644);
+module_param(sysrq_signalled_parent, charp, 0644);
+module_param(sysrq_signal, charp, 0644);
+
 #else
 
+static void sysrq_signal_configured(int key)
+{
+}
+
 static inline void sysrq_register_handler(void)
 {
 }
-- 
2.17.1

Re: [PATCH 5/6] tty/sysrq: Add configurable handler to signal a process

[Greg Kroah-Hartman]

On Mon, May 11, 2020 at 03:59:17PM +0200, Andrzej Pietrasiewicz wrote:
> Some userland might want to implement a policy to signal a configured
> process with a configured signal. This patch adds necessary kernel-side
> infrastructure and the newly added handler is triggered with
> Alt-Shift-SysRq-s. Optionally the userland can also specify the expected
> name of parent process of the victim.

THat's crazy, what "userspace" wants to do something like this that
can't just do it by running a program?  Why force the kernel to do it
for them?

And you don't document any of this :(

greg k-h

Re: [PATCH 5/6] tty/sysrq: Add configurable handler to signal a process

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

Hi Andrzej,

I love your patch! Yet something to improve:

[auto build test ERROR on 2ef96a5bb12be62ef75b5828c0aab838ebb29cb8]

url:    https://github.com/0day-ci/linux/commits/Andrzej-Pietrasiewicz/Magic-SysRq-extensions/20200514-151142
base:    2ef96a5bb12be62ef75b5828c0aab838ebb29cb8
config: alpha-defconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/tty/sysrq.c: In function 'sysrq_str_to_signal':
>> drivers/tty/sysrq.c:791:17: error: 'SIGSTKFLT' undeclared (first use in this function); did you mean 'SIGSTKSZ'?
791 |   {"SIGSTKFLT", SIGSTKFLT},
|                 ^~~~~~~~~
|                 SIGSTKSZ
drivers/tty/sysrq.c:791:17: note: each undeclared identifier is reported only once for each function it appears in

vim +791 drivers/tty/sysrq.c

   771	
   772	static void sysrq_str_to_signal(void)
   773	{
   774		static const struct signal_search signals[] = {
   775			{"SIGHUP", SIGHUP},
   776			{"SIGINT", SIGINT},
   777			{"SIGQUIT", SIGQUIT},
   778			{"SIGILL", SIGILL},
   779			{"SIGTRAP", SIGTRAP},
   780			{"SIGABRT", SIGABRT},
   781			{"SIGIOT", SIGIOT},
   782			{"SIGBUS", SIGBUS},
   783			{"SIGFPE", SIGFPE},
   784			{"SIGKILL", SIGKILL},
   785			{"SIGUSR1", SIGUSR1},
   786			{"SIGSEGV", SIGSEGV},
   787			{"SIGUSR2", SIGUSR2},
   788			{"SIGPIPE", SIGPIPE},
   789			{"SIGALRM", SIGALRM},
   790			{"SIGTERM", SIGTERM},
 > 791			{"SIGSTKFLT", SIGSTKFLT},
   792			{"SIGCHLD", SIGCHLD},
   793			{"SIGCONT", SIGCONT},
   794			{"SIGSTOP", SIGSTOP},
   795			{"SIGTSTP", SIGTSTP},
   796			{"SIGTTIN", SIGTTIN},
   797			{"SIGTTOU", SIGTTOU},
   798			{"SIGURG", SIGURG},
   799			{"SIGXCPU", SIGXCPU},
   800			{"SIGXFSZ", SIGXFSZ},
   801			{"SIGVTALRM", SIGVTALRM},
   802			{"SIGPROF", SIGPROF},
   803			{"SIGWINCH", SIGWINCH},
   804			{"SIGIO", SIGIO},
   805			{"SIGPOLL", SIGPOLL},
   806			{"SIGPWR", SIGPWR},
   807			{"SIGSYS", SIGSYS},
   808		};
   809		int i;
   810	
   811		if (!sysrq_signal)
   812			return;
   813	
   814		for (i = 0; i < ARRAY_SIZE(signals); ++i)
   815			if (!strcmp(signals[i].name, sysrq_signal))
   816				break;
   817	
   818		if (i >= ARRAY_SIZE(signals)) {
   819			pr_err("Unknown signal name %s", sysrq_signal);
   820	
   821			return;
   822		}
   823	
   824		sysrq_signal_code = signals[i].code;
   825	}
   826	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13719 bytes --]

[PATCH 6/6] tty/sysrq: Add configurable handler to execute a compound action

[Andrzej Pietrasiewicz]

Some userland might want to execute e.g. 'w' (show blocked tasks), followed
by 's' (sync), followed by 1000 ms delay and then followed by 'c' (crash)
upon a single magic SysRq. Or one might want to execute the famous "Raising
Elephants Is So Utterly Boring" action. This patch adds a configurable
handler, triggered with 'C', for this exact purpose. The user specifies the
composition of the compound action using syntax similar to getopt, where
each letter corresponds to an individual action and a colon followed by a
number corresponds to a delay of that many milliseconds, e.g.:

ws:1000c

or

r:100eis:1000ub

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/tty/sysrq.c | 73 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index a6e91e4ae304..bde8de2d5b17 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -19,6 +19,7 @@
 #include <linux/sched/rt.h>
 #include <linux/sched/debug.h>
 #include <linux/sched/task.h>
+#include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/mm.h>
 #include <linux/fs.h>
@@ -446,6 +447,15 @@ static struct sysrq_key_op sysrq_signal_configured_op = {
 	.enable_mask	= SYSRQ_ENABLE_SIGNAL,
 };
 
+static void sysrq_action_compound(int key);
+
+static struct sysrq_key_op sysrq_action_compound_op = {
+	.handler	= sysrq_action_compound,
+	.help_msg	= "execute-compound-action(C)",
+	.action_msg	= "Execute compound action",
+	.enable_mask	= SYSRQ_ENABLE_SIGNAL,
+};
+
 /* Key Operations table and lock */
 static DEFINE_SPINLOCK(sysrq_key_table_lock);
 
@@ -508,7 +518,7 @@ static struct sysrq_key_op *sysrq_key_table[62] = {
 	&sysrq_ftrace_dump_op,		/* z */
 	NULL,				/* A */
 	NULL,				/* B */
-	NULL,				/* C */
+	&sysrq_action_compound_op,	/* C */
 	NULL,				/* D */
 	NULL,				/* E */
 	NULL,				/* F */
@@ -646,6 +656,7 @@ static char *sysrq_signalled;
 static char *sysrq_signalled_parent;
 static char *sysrq_signal;
 static int sysrq_signal_code;
+static char *sysrq_compound_action;
 
 /* Simple translation table for the SysRq keys */
 static const unsigned char sysrq_xlate[KEY_CNT] =
@@ -864,6 +875,61 @@ static void sysrq_signal_configured(int key)
 	read_unlock(&tasklist_lock);
 }
 
+#define SYSRQ_COMPOUND_ACTION_VALIDATE	0
+#define SYSRQ_COMPOUND_ACTION_RUN	1
+
+static int sysrq_process_compound_action(int pass)
+{
+	const char *action = sysrq_compound_action;
+	struct sysrq_key_op *op_p;
+	int ret, delay;
+
+	while (*action) {
+		op_p = __sysrq_get_key_op(*action);
+		if (!op_p)
+			return -EINVAL;
+
+		/* Don't allow calling ourselves recursively */
+		if (op_p == &sysrq_action_compound_op)
+			return -EINVAL;
+
+		if (pass == SYSRQ_COMPOUND_ACTION_RUN)
+			__handle_sysrq(*action, false);
+
+		if (*++action == ':') {
+			ret = sscanf(action++, ":%d", &delay);
+			if (ret < 1) /* we want at least ":[0-9]" => 1 item */
+				return -EINVAL;
+
+			while (*action >= '0' && *action <= '9')
+				++action;
+			if (pass == SYSRQ_COMPOUND_ACTION_RUN)
+				mdelay(delay);
+		}
+	}
+
+	return 0;
+}
+
+static void sysrq_action_compound(int key)
+{
+	if (!sysrq_compound_action) {
+		pr_err("Unconfigured compound action for %s",
+		       sysrq_action_compound_op.help_msg);
+
+		return;
+	}
+
+	if (sysrq_process_compound_action(SYSRQ_COMPOUND_ACTION_VALIDATE)) {
+		pr_err("Incorrect compound action %s for %s",
+		       sysrq_compound_action,
+		       sysrq_action_compound_op.help_msg);
+
+		return;
+	}
+
+	sysrq_process_compound_action(SYSRQ_COMPOUND_ACTION_RUN);
+}
 
 static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 {
@@ -1165,12 +1231,17 @@ module_param(sysrq_signalled, charp, 0644);
 module_param(sysrq_signalled_parent, charp, 0644);
 module_param(sysrq_signal, charp, 0644);
 
+module_param(sysrq_compound_action, charp, 0644);
 #else
 
 static void sysrq_signal_configured(int key)
 {
 }
 
+static void sysrq_action_compound(int key)
+{
+}
+
 static inline void sysrq_register_handler(void)
 {
 }
-- 
2.17.1

Re: [PATCH 6/6] tty/sysrq: Add configurable handler to execute a compound action

[Greg Kroah-Hartman]

On Mon, May 11, 2020 at 03:59:18PM +0200, Andrzej Pietrasiewicz wrote:
> Some userland might want to execute e.g. 'w' (show blocked tasks), followed
> by 's' (sync), followed by 1000 ms delay and then followed by 'c' (crash)
> upon a single magic SysRq. Or one might want to execute the famous "Raising
> Elephants Is So Utterly Boring" action. This patch adds a configurable
> handler, triggered with 'C', for this exact purpose. The user specifies the
> composition of the compound action using syntax similar to getopt, where
> each letter corresponds to an individual action and a colon followed by a
> number corresponds to a delay of that many milliseconds, e.g.:
> 
> ws:1000c
> 
> or
> 
> r:100eis:1000ub

Cute, but why?  Who needs/wants this type of thing?

And again, no documentation :(

greg k-h

Re: [PATCH 6/6] tty/sysrq: Add configurable handler to execute a compound action

[Dmitry Torokhov]

On Mon, May 11, 2020 at 06:21:13PM +0200, Greg Kroah-Hartman wrote:
> On Mon, May 11, 2020 at 03:59:18PM +0200, Andrzej Pietrasiewicz wrote:
> > Some userland might want to execute e.g. 'w' (show blocked tasks), followed
> > by 's' (sync), followed by 1000 ms delay and then followed by 'c' (crash)
> > upon a single magic SysRq. Or one might want to execute the famous "Raising
> > Elephants Is So Utterly Boring" action. This patch adds a configurable
> > handler, triggered with 'C', for this exact purpose. The user specifies the
> > composition of the compound action using syntax similar to getopt, where
> > each letter corresponds to an individual action and a colon followed by a
> > number corresponds to a delay of that many milliseconds, e.g.:
> > 
> > ws:1000c
> > 
> > or
> > 
> > r:100eis:1000ub
> 
> Cute, but why?  Who needs/wants this type of thing?

On Chrome OS the first time user presses SysRq-X it will try to kill
chrome (and that will cause crash to get uploaded if user consented).
The 2nd time within 5 seconds the same combo is pressed, it will dump
blocked tasks in syslog and try to sync and then panic. On panic the
device will reboot, logs will be scraped from pstore, and uploaded for
analysis.

Thanks.

-- 
Dmitry

Re: [PATCH 6/6] tty/sysrq: Add configurable handler to execute a compound action

[Andrzej Pietrasiewicz]

Hi,

W dniu 11.05.2020 o 20:29, Dmitry Torokhov pisze:
> On Mon, May 11, 2020 at 06:21:13PM +0200, Greg Kroah-Hartman wrote:
>> On Mon, May 11, 2020 at 03:59:18PM +0200, Andrzej Pietrasiewicz wrote:
>>> Some userland might want to execute e.g. 'w' (show blocked tasks), followed
>>> by 's' (sync), followed by 1000 ms delay and then followed by 'c' (crash)
>>> upon a single magic SysRq. Or one might want to execute the famous "Raising
>>> Elephants Is So Utterly Boring" action. This patch adds a configurable
>>> handler, triggered with 'C', for this exact purpose. The user specifies the
>>> composition of the compound action using syntax similar to getopt, where
>>> each letter corresponds to an individual action and a colon followed by a
>>> number corresponds to a delay of that many milliseconds, e.g.:
>>>
>>> ws:1000c
>>>
>>> or
>>>
>>> r:100eis:1000ub
>>
>> Cute, but why?  Who needs/wants this type of thing?

Surely things that can be done in userspace should be done there.
So we would envision an input daemon which reacts to a predefined
combination of keys. That said, it is not unimaginable to think of
userspace being dead enough (e.g. due to memory pressure) to be unable
to complete such a compound action. In other words userspace not being
able to do it is a good reason for putting the code in the kernel.

Dmitry has given a use case where such a compound action is needed.

Andrzej

> 
> On Chrome OS the first time user presses SysRq-X it will try to kill
> chrome (and that will cause crash to get uploaded if user consented).
> The 2nd time within 5 seconds the same combo is pressed, it will dump
> blocked tasks in syslog and try to sync and then panic. On panic the
> device will reboot, logs will be scraped from pstore, and uploaded for
> analysis.
> 
> Thanks.
>