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

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

[Tomas Mudrunka]

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;`

Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
---
 drivers/tty/sysrq.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 6b4a28bcf..bc5a679f6 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -1154,10 +1154,12 @@ EXPORT_SYMBOL(unregister_sysrq_key);
 static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
 				   size_t count, loff_t *ppos)
 {
-	if (count) {
+	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);
 	}
-- 
2.42.1

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

[Greg Kroah-Hartman]

On Mon, Nov 13, 2023 at 07:22:19PM +0100, 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;`
> 
> Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
> ---
>  drivers/tty/sysrq.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 6b4a28bcf..bc5a679f6 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -1154,10 +1154,12 @@ EXPORT_SYMBOL(unregister_sysrq_key);
>  static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
>  				   size_t count, loff_t *ppos)
>  {
> -	if (count) {
> +	size_t i;
> +
> +	for (i = 0; i < count; i++) {
>  		char c;
>  
> -		if (get_user(c, buf))
> +		if (get_user(c, buf+i))

What did you just break where people would send a string and only relied
on the first character being checked?  This might not be ok to do.

Also, no documentation update?

thanks,

greg k-h

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

[Tomáš Mudruňka]

po 13. 11. 2023 v 19:33 Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> What did you just break where people would send a string and only relied
> on the first character being checked?  This might not be ok to do.

It truly might not be ok. But i hope you will not consider me impolite
if i open further discussion. Is this actual use case for some people?
I understand that it might be, but currently i can only think about
this being done by mistake, rather than on purpose... are you aware of
any software actually leveraging this feature?

Please also note that there is really good use case for this. if i do
the REISUB bash loop as suggested, the bash will be killed during E or
I and the rest of emergency procedure will not finish. Therefore i
really think it makes sense to be able to pass whole sysrq batch to
the kernel at once.

In case you are sure that this is a bad idea, i can suggest
alternative approach. only activate the "bulk mode" when first
character is '_' (underscore).
User would then be able to do
echo _reisub > /proc/sysrq-trigger

Would you prefer if i do it this way? In my opinion it does introduce
little unnecessary complexity, to fix something that might arguably
not be actual issue. I mean... we still have /dev/null in case people
need to discard some extra characters. :-)
But if you think it's better to stay on safe side, i think it's viable
option as well...

> Also, no documentation update?

Will fix this one in v2.

> thanks,
>
> greg k-h

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

[Greg Kroah-Hartman]

On Mon, Nov 13, 2023 at 10:09:50PM +0100, Tomáš Mudruňka wrote:
> po 13. 11. 2023 v 19:33 Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > What did you just break where people would send a string and only relied
> > on the first character being checked?  This might not be ok to do.
> 
> It truly might not be ok. But i hope you will not consider me impolite
> if i open further discussion. Is this actual use case for some people?

It might be, we can't start doing different functionality just because
we were previously ignoring all the other characters.  Especially for a
user/kernel api that is allowed to reboot the machine :)

> I understand that it might be, but currently i can only think about
> this being done by mistake, rather than on purpose... are you aware of
> any software actually leveraging this feature?

I'm not, but there is a lot of userspace software out there...

> Please also note that there is really good use case for this. if i do
> the REISUB bash loop as suggested, the bash will be killed during E or
> I and the rest of emergency procedure will not finish. Therefore i
> really think it makes sense to be able to pass whole sysrq batch to
> the kernel at once.
> 
> In case you are sure that this is a bad idea, i can suggest
> alternative approach. only activate the "bulk mode" when first
> character is '_' (underscore).
> User would then be able to do
> echo _reisub > /proc/sysrq-trigger
> 
> Would you prefer if i do it this way? In my opinion it does introduce
> little unnecessary complexity, to fix something that might arguably
> not be actual issue. I mean... we still have /dev/null in case people
> need to discard some extra characters. :-)
> But if you think it's better to stay on safe side, i think it's viable
> option as well...

I think you are only going to be able to do this "mode switch" method as
we can't potentially break existing systems.  Your change should prevent
that from happening, so at a quick glance, yes, this seems like the only
way forward if this is needed at all (an independent question...)

thanks,

greg k-h

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

[Tomas Mudrunka]

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>
---
 Documentation/admin-guide/sysrq.rst |  4 ++++
 drivers/tty/sysrq.c                 | 17 ++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..9d46a33e5 100644
--- a/Documentation/admin-guide/sysrq.rst
+++ b/Documentation/admin-guide/sysrq.rst
@@ -79,6 +79,10 @@ On all
 
 		echo t > /proc/sysrq-trigger
 
+	Alternatively write key combo prepended by underscore. 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..3455e6dd3 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) {
+	char 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;
-- 
2.42.1

Re: [PATCH v2] /proc/sysrq-trigger: accept multiple keys at once

[Jiri Slaby]

On 14. 11. 23, 10:35, 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>
> ---
>   Documentation/admin-guide/sysrq.rst |  4 ++++
>   drivers/tty/sysrq.c                 | 17 ++++++++++++++---
>   2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
> index 51906e473..9d46a33e5 100644
> --- a/Documentation/admin-guide/sysrq.rst
> +++ b/Documentation/admin-guide/sysrq.rst
> @@ -79,6 +79,10 @@ On all
>   
>   		echo t > /proc/sysrq-trigger
>   
> +	Alternatively write key combo prepended by underscore. 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..3455e6dd3 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) {
> +	char bulk = false;

bool

> +	size_t i;
> +
> +	for (i = 0; i < count; i++) {
>   		char c;
>   
> -		if (get_user(c, buf))
> +		if (get_user(c, buf+i))

spaces around +

>   			return -EFAULT;
> -		__handle_sysrq(c, false);
> +
> +		if (c == '_')
> +			bulk = true;
> +		else
> +			__handle_sysrq(c, false);
> +
> +		if (!bulk)
> +			break;
>   	}
>   
>   	return count;

-- 
js
suse labs

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

[Tomas Mudrunka]

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>
---
 Documentation/admin-guide/sysrq.rst |  4 ++++
 drivers/tty/sysrq.c                 | 19 +++++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..9d46a33e5 100644
--- a/Documentation/admin-guide/sysrq.rst
+++ b/Documentation/admin-guide/sysrq.rst
@@ -79,6 +79,10 @@ On all
 
 		echo t > /proc/sysrq-trigger
 
+	Alternatively write key combo prepended by underscore. 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..367028b1e 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) {
-		char c;
+	char bulk = false;
+	size_t i;
 
-		if (get_user(c, buf))
+	for (i = 0; i < count; i++) {
+		bool c;
+
+		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;
-- 
2.42.1

Re: [PATCH v3] /proc/sysrq-trigger: accept multiple keys at once

[Jiri Slaby]

On 14. 11. 23, 10:49, 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>
> ---

What has changed in v2, v3?

>   Documentation/admin-guide/sysrq.rst |  4 ++++
>   drivers/tty/sysrq.c                 | 19 +++++++++++++++----
>   2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
> index 51906e473..9d46a33e5 100644
> --- a/Documentation/admin-guide/sysrq.rst
> +++ b/Documentation/admin-guide/sysrq.rst
> @@ -79,6 +79,10 @@ On all
>   
>   		echo t > /proc/sysrq-trigger
>   
> +	Alternatively write key combo prepended by underscore. 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..367028b1e 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) {
> -		char c;
> +	char bulk = false;
> +	size_t i;
>   
> -		if (get_user(c, buf))
> +	for (i = 0; i < count; i++) {
> +		bool c;

bool c?


-- 
js
suse labs

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

[Tomas Mudrunka]

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>
---
 Documentation/admin-guide/sysrq.rst |  4 ++++
 drivers/tty/sysrq.c                 | 17 ++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..9d46a33e5 100644
--- a/Documentation/admin-guide/sysrq.rst
+++ b/Documentation/admin-guide/sysrq.rst
@@ -79,6 +79,10 @@ On all
 
 		echo t > /proc/sysrq-trigger
 
+	Alternatively write key combo prepended by underscore. 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;
-- 
2.42.1

Re: [PATCH v4] /proc/sysrq-trigger: accept multiple keys at once

[Greg KH]

On Tue, Nov 14, 2023 at 10:55:57AM +0100, 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>
> ---
>  Documentation/admin-guide/sysrq.rst |  4 ++++
>  drivers/tty/sysrq.c                 | 17 ++++++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

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

[Tomas Mudrunka]

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>
---
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 |  4 ++++
 drivers/tty/sysrq.c                 | 17 ++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..9d46a33e5 100644
--- a/Documentation/admin-guide/sysrq.rst
+++ b/Documentation/admin-guide/sysrq.rst
@@ -79,6 +79,10 @@ On all
 
 		echo t > /proc/sysrq-trigger
 
+	Alternatively write key combo prepended by underscore. 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;
-- 
2.42.1

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

[Tomas Mudrunka]

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.
+	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;
-- 
2.42.1

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

[Jiri Slaby]

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,
-- 
js
suse labs

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

[Randy Dunlap]

On 11/14/23 10:04, Jiri Slaby wrote:
> 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?
> 

or Randy?
Yes, you are correct.
All of your recommendations look good.
Thanks.

>>             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,

-- 
~Randy

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

[Jiri Slaby]

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.

-- 
js
suse labs

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

[Tomas Mudrunka]

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>
---
 Documentation/admin-guide/sysrq.rst | 11 ++++++++++-
 drivers/tty/sysrq.c                 | 18 +++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..fbe79d314 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 the first character is processed, the rest of string is ignored.
+	However, it is not recommended 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 characters prepended by underscore.
+	This way, all characters will be processed. 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..5411351e4 100644
--- 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.
+ * If first character is underscore, all characters are processed.
  */
 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;
-- 
2.42.1

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

[Tomas Mudrunka]

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>
---
V7 -> V8: Added this list of changes
V6 -> V7: Fixed english in documentation
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                 | 18 +++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..fbe79d314 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 the first character is processed, the rest of string is ignored.
+	However, it is not recommended 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 characters prepended by underscore.
+	This way, all characters will be processed. 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..5411351e4 100644
--- 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.
+ * If first character is underscore, all characters are processed.
  */
 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;
-- 
2.42.1

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

[Tomas Mudrunka]

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
V7 -> V8: Added this list of changes
V6 -> V7: Fixed english in documentation
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                 | 18 +++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..0042fa26b 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 a single character to /proc/sysrq-trigger.
+	Only the first character is processed, the rest of string is ignored.
+	However, it is not recommended 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 characters prepended by underscore.
+	This way, all characters will be processed. 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..5411351e4 100644
--- 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.
+ * If first character is underscore, all characters are processed.
  */
 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;
-- 
2.42.1

Re: [PATCH v9] /proc/sysrq-trigger: accept multiple keys at once

[Jiri Slaby]

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,
-- 
js
suse labs

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

[Tomas Mudrunka]

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 (eg. bash or ssh) 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>
---
V9 -> V10: More english fixes
V8 -> V9: Fixed english bit more
V7 -> V8: Added this list of changes
V6 -> V7: Fixed english in documentation
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                 | 19 ++++++++++++++++---
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 51906e473..2f2e5bd44 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 a single character to /proc/sysrq-trigger.
+	Only the first character is processed, the rest of the string is
+	ignored. However, it is not recommended 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 characters prepended by underscore.
+	This way, all characters will be processed. 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..02217e3c9 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -1150,16 +1150,29 @@ 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.
+ * However, if the first character is an underscore,
+ * all characters are processed.
  */
 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;
-- 
2.42.1

Re: [PATCH v10] /proc/sysrq-trigger: accept multiple keys at once

[Jiri Slaby]

On 20. 11. 23, 12:14, Tomas Mudrunka wrote:
> 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 (eg. bash or ssh) 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>

Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

thanks,
-- 
js
suse labs