Re: [PATCH v4 1/2] tty: sysrq: don't expose a partially written reset_seq

[Greg KH]

On Fri, Oct 30, 2026 at 12:58:53PM +0800, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
> 
> reset_seq is a module_param_array, so param_array() builds it up one
> element at a time: it resets the element count, then for each element
> calls sysrq_reset_seq_param_set(), which stores the key and bumps
> sysrq_reset_seq_version, and afterwards increments the count.  Every
> intermediate step is a self-consistent sequence carrying a fresh version,
> and sysrq_handle_keypress() reparses as soon as the version moves, so
> writing 56,29,111 publishes a one-key sequence and then a two-key one
> before the intended three-key one.
> 
> A key event that lands on the one-key state gives the device a one-key
> sequence, and because the parse runs before set_bit(code, key_down) the
> count is still 0 and reset_canceled is false.  The same event then reaches
> sysrq_detect_reset_sequence(), where ++reset_seq_cnt matches reset_seq_len
> and sysrq_handle_reset_request() runs; sysrq_reset_downtime_ms defaults to
> 0, so sysrq_do_reset() calls orderly_reboot() straight away.  Writing the
> parameter needs root and the key has to be the first one of the new
> sequence, but an administrator who presses that key while reconfiguring
> the sequence reboots the machine.
> 
> Today this needs a device that has not parsed a sequence yet, because
> reset_seq_len only ever grows.  The next patch resets it on every reparse,
> which without this one would expose every device.
> 
> Parse the whole parameter in one callback and replace the sequence under
> a seqlock.  The reader copies it out under raw_seqcount_try_begin(), as
> it runs from the input event path with interrupts disabled, and keeps
> its current sequence if a writer got in the way.  It is a seqlock rather
> than a bare seqcount because writers were not serialised either:
> sysrq_of_get_keyreset_config() stores the sequence from the kernel.sysrq
> sysctl without the module parameter lock.

I'm sorry, but I can't really parse this llm-generated text.  What is
the bug here.  What is being fixed?  And what is changing?

> Behaviour changes:
>  - A key of 0 is rejected instead of silently ending the sequence there.
>  - A write that fails partway keeps the previous sequence.
>  - An element longer than seven characters, such as a zero-padded hex
>    value, is rejected.

So what workflow hits this and will need to be changed if this patch is
applied?

> Fixes: 154b7a489a5b ("Input: sysrq - allow specifying alternate reset sequence")

no cc: stable?

And how has no one ever hit this issue in the decades that it has been
present?  Is this really something that needs to be fixed?  If so, why?

thanks,

greg k-h