diff mbox series

[v6] /proc/sysrq-trigger: accept multiple keys at once

Message ID 20231114151252.117575-1-tomas.mudrunka@gmail.com
State Superseded
Headers show
Series [v6] /proc/sysrq-trigger: accept multiple keys at once | expand

Commit Message

Tomas Mudrunka Nov. 14, 2023, 3:12 p.m. UTC
Just for convenience.
This way we can do:
`echo _reisub > /proc/sysrq-trigger`
Instead of:
`for i in r e i s u b; do echo "$i" > /proc/sysrq-trigger; done;`

This can be very useful when trying to execute sysrq combo remotely
or from userspace. When sending keys in multiple separate writes,
userspace can be killed before whole combo is completed.
Therefore putting all keys in single write is more robust approach.

Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
---
V5 -> V6: Documentation now has notice about undefined behavior
V4 -> V5: Added this list of changes
V3 -> V4: Bulk is now bool instead of char (and fixed typo)
V2 -> V3: Fixed code styling (and introduced typo)
V1 -> V2: Bulk mode only activated by underscore now, added docs

 Documentation/admin-guide/sysrq.rst | 11 ++++++++++-
 drivers/tty/sysrq.c                 | 17 ++++++++++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

Comments

Jiri Slaby Nov. 14, 2023, 6:04 p.m. UTC | #1
On 14. 11. 23, 16:12, Tomas Mudrunka wrote:
> Just for convenience.
> This way we can do:
> `echo _reisub > /proc/sysrq-trigger`
> Instead of:
> `for i in r e i s u b; do echo "$i" > /proc/sysrq-trigger; done;`
> 
> This can be very useful when trying to execute sysrq combo remotely
> or from userspace. When sending keys in multiple separate writes,
> userspace can be killed before whole combo is completed.
> Therefore putting all keys in single write is more robust approach.
> 
> Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
> ---
> V5 -> V6: Documentation now has notice about undefined behavior
> V4 -> V5: Added this list of changes
> V3 -> V4: Bulk is now bool instead of char (and fixed typo)
> V2 -> V3: Fixed code styling (and introduced typo)
> V1 -> V2: Bulk mode only activated by underscore now, added docs
> 
>   Documentation/admin-guide/sysrq.rst | 11 ++++++++++-
>   drivers/tty/sysrq.c                 | 17 ++++++++++++++---
>   2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
> index 51906e473..e7a82cba7 100644
> --- a/Documentation/admin-guide/sysrq.rst
> +++ b/Documentation/admin-guide/sysrq.rst
> @@ -75,10 +75,19 @@ On other
>   	submit a patch to be included in this section.
>   
>   On all
> -	Write a character to /proc/sysrq-trigger.  e.g.::
> +	Write single character to /proc/sysrq-trigger.

a single

> +	Only first character is interpreted, rest of string is ignored.

the first; the rest of the string

> +	However it is not reccomended to write any extra characters

However, <- comma
recommended

> +	as the behavior is undefined and might change in the future versions.
> +	e.g.::

Even the original was lowercase. But it should be "E.g.::", right -- 
Greg/Jon?

>   
>   		echo t > /proc/sysrq-trigger
>   
> +	Alternatively write multiple keys combo prepended by underscore.

Alternatively, <- comma
s/keys/characters/
an underscore

> +	All characters are interpreted this way. e.g.::

This way, all characters are interpreted. (IMO this has a different 
meaning, but natives would have to tell us.)

> +
> +		echo _reisub > /proc/sysrq-trigger
> +
>   The :kbd:`<command key>` is case sensitive.
>   
>   What are the 'command' keys?
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 6b4a28bcf..ad07bc812 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -1150,16 +1150,27 @@ EXPORT_SYMBOL(unregister_sysrq_key);
>   #ifdef CONFIG_PROC_FS
>   /*
>    * writing 'C' to /proc/sysrq-trigger is like sysrq-C
> + * If first character in write is underscore, all characters are interpreted.

If the first character written is

thanks,
Jiri Slaby Nov. 15, 2023, 10:10 a.m. UTC | #2
On 14. 11. 23, 23:00, Randy Dunlap wrote:
>>> +    as the behavior is undefined and might change in the future versions.
>>> +    e.g.::
>>
>> Even the original was lowercase. But it should be "E.g.::", right -- Greg/Jon?
>>
> 
> or Randy?

Sure, thanks for stepping in.
Jiri Slaby Nov. 20, 2023, 7:45 a.m. UTC | #3
On 15. 11. 23, 11:34, Tomas Mudrunka wrote:
> Just for convenience.
> This way we can do:
> `echo _reisub > /proc/sysrq-trigger`
> Instead of:
> `for i in r e i s u b; do echo "$i" > /proc/sysrq-trigger; done;`
> 
> This can be very useful when trying to execute sysrq combo remotely
> or from userspace. When sending keys in multiple separate writes,
> userspace can be killed before whole combo is completed.
> Therefore putting all keys in single write is more robust approach.
> 
> Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
> ---
> V8 -> V9: Fixed english bit more

Please check my comments to v6 more carefully once again. Plus:

> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -1150,16 +1150,28 @@ EXPORT_SYMBOL(unregister_sysrq_key);
>   #ifdef CONFIG_PROC_FS
>   /*
>    * writing 'C' to /proc/sysrq-trigger is like sysrq-C
> + * Normally only the first character written is processed.

Normally, <-- comma

> + * If first character is underscore, all characters are processed.

the first
an underscore

Maybe it would make sense to prepend "However, " to this very sentence?

thanks,
diff mbox series

Patch

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..e7a82cba7 100644
--- a/Documentation/admin-guide/sysrq.rst
+++ b/Documentation/admin-guide/sysrq.rst
@@ -75,10 +75,19 @@  On other
 	submit a patch to be included in this section.
 
 On all
-	Write a character to /proc/sysrq-trigger.  e.g.::
+	Write single character to /proc/sysrq-trigger.
+	Only first character is interpreted, rest of string is ignored.
+	However it is not reccomended to write any extra characters
+	as the behavior is undefined and might change in the future versions.
+	e.g.::
 
 		echo t > /proc/sysrq-trigger
 
+	Alternatively write multiple keys combo prepended by underscore.
+	All characters are interpreted this way. e.g.::
+
+		echo _reisub > /proc/sysrq-trigger
+
 The :kbd:`<command key>` is case sensitive.
 
 What are the 'command' keys?
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 6b4a28bcf..ad07bc812 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -1150,16 +1150,27 @@  EXPORT_SYMBOL(unregister_sysrq_key);
 #ifdef CONFIG_PROC_FS
 /*
  * writing 'C' to /proc/sysrq-trigger is like sysrq-C
+ * If first character in write is underscore, all characters are interpreted.
  */
 static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
 				   size_t count, loff_t *ppos)
 {
-	if (count) {
+	bool bulk = false;
+	size_t i;
+
+	for (i = 0; i < count; i++) {
 		char c;
 
-		if (get_user(c, buf))
+		if (get_user(c, buf + i))
 			return -EFAULT;
-		__handle_sysrq(c, false);
+
+		if (c == '_')
+			bulk = true;
+		else
+			__handle_sysrq(c, false);
+
+		if (!bulk)
+			break;
 	}
 
 	return count;