2023-11-15 10:12:00

by Jiri Slaby

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

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


2023-11-15 10:28:17

by Tomáš Mudruňka

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

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 <[email protected]>
---
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

2023-11-15 10:29:40

by Tomáš Mudruňka

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

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 <[email protected]>
---
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

2023-11-15 10:34:35

by Tomáš Mudruňka

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

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 <[email protected]>
---
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

2023-11-20 07:46:03

by Jiri Slaby

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

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 <[email protected]>
> ---
> 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

2023-11-20 11:15:35

by Tomáš Mudruňka

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

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 <[email protected]>
---
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

2023-11-20 11:33:09

by Jiri Slaby

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

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 <[email protected]>

Reviewed-by: Jiri Slaby <[email protected]>

thanks,
--
js
suse labs