[PATCH] Input: sysrq - reset the per-device key state when reparsing reset_seq

[PATCH] Input: sysrq - reset the per-device key state when reparsing reset_seq

[Zhan Xusheng]

sysrq_parse_reset_sequence() rebuilds a device's view of the reset
sequence, but only clears reset_seq_cnt.  reset_keybit is only ever
__set_bit() and reset_seq_len is only ever incremented, and nothing else
in the file clears either.  The struct sysrq_state holding them is
allocated once per input device in sysrq_connect() and lives until the
device goes away, so both accumulate across calls.

The function is called from sysrq_filter() whenever
sysrq_reset_seq_version has moved on, which happens when the reset_seq
module parameter is written.  It is mode 0644, so this is reachable at
runtime:

  # echo 56,29,111 > /sys/module/sysrq/parameters/reset_seq
  # echo 59,60     > /sys/module/sysrq/parameters/reset_seq

After the second write reset_seq_len is 5 rather than 2, and
reset_keybit has five keys set rather than two.  The trigger in
sysrq_detect_reset_sequence() is

	if (++state->reset_seq_cnt == state->reset_seq_len &&
	    !state->reset_canceled)

and reset_seq_cnt cannot exceed the number of keys actually in the
sequence, so holding the whole new sequence no longer resets the machine.
The stale bits also make keys that were dropped from the sequence still
test positive, so pressing one takes the increment path instead of
cancelling, and releasing one decrements reset_seq_cnt, which can take it
negative.

The boot path is not affected: both the command line and
sysrq_of_get_keyreset_config() leave sysrq_reset_seq_version at its
initial 1, so a freshly kzalloc'd state parses exactly once with both
fields already zero.

Clear them along with reset_seq_cnt.

Note the error direction is towards not resetting, as reset_seq_len only
grows, so this costs an emergency reset on systems that reconfigure the
sequence rather than causing an unwanted one.

Fixes: 154b7a489a5b ("Input: sysrq - allow specifying alternate reset sequence")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 drivers/tty/sysrq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index c2e4b31b699aa..d28a4726b07fc 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -689,6 +689,8 @@ static void sysrq_parse_reset_sequence(struct sysrq_state *state)
 	int i;
 	unsigned short key;
 
+	bitmap_zero(state->reset_keybit, KEY_CNT);
+	state->reset_seq_len = 0;
 	state->reset_seq_cnt = 0;
 
 	for (i = 0; i < sysrq_reset_seq_len; i++) {

base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e
-- 
2.43.0

Re: [PATCH] Input: sysrq - reset the per-device key state when reparsing reset_seq

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tty: sysrq: uncancelable reset timer on sequence change
--

commit 2166ad442fbe41cfd64e5275427188db06092a2e
Author: Zhan Xusheng <zhanxusheng1024@gmail.com>

Input: sysrq - reset the per-device key state when reparsing reset_seq

This commit clears the accumulated reset_seq_len and reset_keybit state when
reparsing the reset sequence. This prevents stale keys from affecting the reset
detection, as the previous code only cleared reset_seq_cnt.

> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index c2e4b31b699aa..d28a4726b07fc 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -689,6 +689,8 @@ static void sysrq_parse_reset_sequence(struct sysrq_state *state)
>  	int i;
>  	unsigned short key;
>  
> +	bitmap_zero(state->reset_keybit, KEY_CNT);
> +	state->reset_seq_len = 0;
>  	state->reset_seq_cnt = 0;
>  
>  	for (i = 0; i < sysrq_reset_seq_len; i++) {

[Severity: Medium]
Does clearing the reset_keybit here create a situation where the reset timer
becomes uncancelable if the sequence changes while keys are held?

If sysrq_reset_downtime_ms is configured and a user is currently holding the
reset sequence keys, the reset timer will be active.  If the sequence is then
modified via sysfs, this function clears the old reset_keybit.

When the user subsequently releases the old keys:

drivers/tty/sysrq.c:sysrq_detect_reset_sequence() {
    if (!test_bit(code, state->reset_keybit)) {
        /* ... */
        if (value && state->reset_seq_cnt) {
            state->reset_canceled = true;
            timer_delete(&state->keyreset_timer);
        }
    } else if (value == 0) {
        ...
}

Because the key is no longer in reset_keybit, it takes the first branch.
Since this is a key release (value == 0), the timer_delete() is skipped,
leaving the timer active and potentially causing an unwanted system reboot.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016032732.2328852-1-zhanxusheng@xiaomi.com?part=1

Re: [PATCH] Input: sysrq - reset the per-device key state when reparsing reset_seq

[Greg Kroah-Hartman]

On Fri, Oct 16, 2026 at 11:27:32AM +0800, Zhan Xusheng wrote:
> sysrq_parse_reset_sequence() rebuilds a device's view of the reset
> sequence, but only clears reset_seq_cnt.  reset_keybit is only ever
> __set_bit() and reset_seq_len is only ever incremented, and nothing else
> in the file clears either.  The struct sysrq_state holding them is
> allocated once per input device in sysrq_connect() and lives until the
> device goes away, so both accumulate across calls.
> 
> The function is called from sysrq_filter() whenever
> sysrq_reset_seq_version has moved on, which happens when the reset_seq
> module parameter is written.  It is mode 0644, so this is reachable at
> runtime:
> 
>   # echo 56,29,111 > /sys/module/sysrq/parameters/reset_seq
>   # echo 59,60     > /sys/module/sysrq/parameters/reset_seq
> 
> After the second write reset_seq_len is 5 rather than 2, and
> reset_keybit has five keys set rather than two.  The trigger in
> sysrq_detect_reset_sequence() is
> 
> 	if (++state->reset_seq_cnt == state->reset_seq_len &&
> 	    !state->reset_canceled)
> 
> and reset_seq_cnt cannot exceed the number of keys actually in the
> sequence, so holding the whole new sequence no longer resets the machine.
> The stale bits also make keys that were dropped from the sequence still
> test positive, so pressing one takes the increment path instead of
> cancelling, and releasing one decrements reset_seq_cnt, which can take it
> negative.
> 
> The boot path is not affected: both the command line and
> sysrq_of_get_keyreset_config() leave sysrq_reset_seq_version at its
> initial 1, so a freshly kzalloc'd state parses exactly once with both
> fields already zero.
> 
> Clear them along with reset_seq_cnt.
> 
> Note the error direction is towards not resetting, as reset_seq_len only
> grows, so this costs an emergency reset on systems that reconfigure the
> sequence rather than causing an unwanted one.
> 
> Fixes: 154b7a489a5b ("Input: sysrq - allow specifying alternate reset sequence")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  drivers/tty/sysrq.c | 2 ++
>  1 file changed, 2 insertions(+)

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:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  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

Re: [PATCH] Input: sysrq - reset the per-device key state when reparsing reset_seq

[Greg Kroah-Hartman]

On Fri, Oct 16, 2026 at 11:27:32AM +0800, Zhan Xusheng wrote:
> sysrq_parse_reset_sequence() rebuilds a device's view of the reset
> sequence, but only clears reset_seq_cnt.  reset_keybit is only ever
> __set_bit() and reset_seq_len is only ever incremented, and nothing else
> in the file clears either.  The struct sysrq_state holding them is
> allocated once per input device in sysrq_connect() and lives until the
> device goes away, so both accumulate across calls.
> 
> The function is called from sysrq_filter() whenever
> sysrq_reset_seq_version has moved on, which happens when the reset_seq
> module parameter is written.  It is mode 0644, so this is reachable at
> runtime:
> 
>   # echo 56,29,111 > /sys/module/sysrq/parameters/reset_seq
>   # echo 59,60     > /sys/module/sysrq/parameters/reset_seq
> 
> After the second write reset_seq_len is 5 rather than 2, and
> reset_keybit has five keys set rather than two.  The trigger in
> sysrq_detect_reset_sequence() is
> 
> 	if (++state->reset_seq_cnt == state->reset_seq_len &&
> 	    !state->reset_canceled)
> 
> and reset_seq_cnt cannot exceed the number of keys actually in the
> sequence, so holding the whole new sequence no longer resets the machine.
> The stale bits also make keys that were dropped from the sequence still
> test positive, so pressing one takes the increment path instead of
> cancelling, and releasing one decrements reset_seq_cnt, which can take it
> negative.
> 
> The boot path is not affected: both the command line and
> sysrq_of_get_keyreset_config() leave sysrq_reset_seq_version at its
> initial 1, so a freshly kzalloc'd state parses exactly once with both
> fields already zero.
> 
> Clear them along with reset_seq_cnt.
> 
> Note the error direction is towards not resetting, as reset_seq_len only
> grows, so this costs an emergency reset on systems that reconfigure the
> sequence rather than causing an unwanted one.
> 
> Fixes: 154b7a489a5b ("Input: sysrq - allow specifying alternate reset sequence")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  drivers/tty/sysrq.c | 2 ++
>  1 file changed, 2 insertions(+)

Why is a tty patch prefix "Input:"?

thanks,

greg k-h