[PATCH] /proc/sysrq-trigger can now pause processing for one second

[PATCH] /proc/sysrq-trigger can now pause processing for one second

[Tomas Mudrunka]

Writing ',' to /proc/sysrq-trigger now causes processing to
pause for one second.

This is useful, because recently accepted patch allows
to write multiple keys at once to /proc/sysrq-trigger.
But it might be desirable to add slight delay between actions.

Eg. between (e)TERM and (i)KILL it makes sense to put slight delay,
so processes have chance to run TERM handlers before being KILLed.

Now we can send TERM, wait for two seconds and KILL like this:

echo _e,,i > /proc/sysrq-trigger

Originaly i've tested doing this as handler registered in
sysrq_key_table[], but that would cause delay to occur while
holding sysrq rcu lock in __handle_sysrq(), therefore i've decided
to implement this in write_sysrq_trigger() instead to allow
proper use of msleep() instead of mdelay() in locked context.

This means it will only be possible to invoke the delay using
/proc/sysrq-trigger, but there is little point in doing that
interactively using keyboard anyway.

Depends-on: /proc/sysrq-trigger: accept multiple keys at once

Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
---
 Documentation/admin-guide/sysrq.rst |  2 ++
 drivers/tty/sysrq.c                 | 16 ++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/sysrq.rst b/Documentation/admin-guide/sysrq.rst
index 2f2e5bd44..b798a2695 100644
--- a/Documentation/admin-guide/sysrq.rst
+++ b/Documentation/admin-guide/sysrq.rst
@@ -161,6 +161,8 @@ Command	    Function
             will be printed to your console. (``0``, for example would make
             it so that only emergency messages like PANICs or OOPSes would
             make it to your console.)
+
+``,``	    Wait for one second (only for use with /proc/sysrq-trigger batch)
 =========== ===================================================================
 
 Okay, so what can I use them for?
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 02217e3c9..a19ce0865 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -51,6 +51,7 @@
 #include <linux/syscalls.h>
 #include <linux/of.h>
 #include <linux/rcupdate.h>
+#include <linux/delay.h>
 
 #include <asm/ptrace.h>
 #include <asm/irq_regs.h>
@@ -1166,10 +1167,21 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
 		if (get_user(c, buf + i))
 			return -EFAULT;
 
-		if (c == '_')
+		switch (c) {
+
+		case '_':
 			bulk = true;
-		else
+			break;
+
+		case ',':
+			msleep(1000);
+			break;
+
+		default:
 			__handle_sysrq(c, false);
+			break;
+
+		}
 
 		if (!bulk)
 			break;
-- 
2.43.0

Re: [PATCH] /proc/sysrq-trigger can now pause processing for one second

[Jiri Slaby]

On 18. 12. 23, 12:42, Tomas Mudrunka wrote:
> Writing ',' to /proc/sysrq-trigger now causes processing to
> pause for one second.
> 
> This is useful, because recently accepted patch allows
> to write multiple keys at once to /proc/sysrq-trigger.
> But it might be desirable to add slight delay between actions.
> 
> Eg. between (e)TERM and (i)KILL it makes sense to put slight delay,
> so processes have chance to run TERM handlers before being KILLed.
> 
> Now we can send TERM, wait for two seconds and KILL like this:
> 
> echo _e,,i > /proc/sysrq-trigger

Bah, what's wrong with:
  echo e > /proc/sysrq-trigger
  sleep 2
  echo i > /proc/sysrq-trigger
?

thanks,
-- 
js
suse labs

Re: [PATCH] /proc/sysrq-trigger can now pause processing for one second

[Tomáš Mudruňka]

> Bah, what's wrong with:
>   echo e > /proc/sysrq-trigger
>   sleep 2
>   echo i > /proc/sysrq-trigger
> ?

Your bash, or even ssh session being killed during the first line, not
getting to the two subsequent lines.

Tomas

Re: [PATCH] /proc/sysrq-trigger can now pause processing for one second

[Greg Kroah-Hartman]

On Mon, Dec 18, 2023 at 01:13:34PM +0100, Tomáš Mudruňka wrote:
> > Bah, what's wrong with:
> >   echo e > /proc/sysrq-trigger
> >   sleep 2
> >   echo i > /proc/sysrq-trigger
> > ?
> 
> Your bash, or even ssh session being killed during the first line, not
> getting to the two subsequent lines.

What will kill it?  I feel like you are adding features to the kernel
that can be done in userspace, which is generally not a good idea.

thanks,

greg k-h

Re: [PATCH] /proc/sysrq-trigger can now pause processing for one second

[Tomáš Mudruňka]

> What will kill it?  I feel like you are adding features to the kernel
> that can be done in userspace, which is generally not a good idea.

The mere act of writing "e" to /proc/sysrq-trigger kills everything
except for init, which is rather unfortunate when doing that through
remote access, like ssh (or other). I can surely block SIGTERM in
userspace by fixing all remote access software that exists to not exit
after SIGTERM, but if i want to do SIGKILL and then execute few more
sysrq actions (sync, unmount, reboot, ...) it surely is a problem
unless i am doing this from init process. which sometimes is just not
possible on remote system that have undergone some crash. and as linux
admin with 13 years of experience i can safely say that situations
with unresponsive init do happen every now and then. that is when i
usually have to resort to rebooting the system remotely via
sysrq-trigger. this process failing can be difference between me being
able to fix issue remotely with minimum downtime and me having to
physicaly visit datacenter during holidays.

BTW if still unclear, here is simple example of how running that
suggested code will not work:

$ ssh root@10.10.10.10
root@10.10.10.10's password:
Last login: Wed Oct  4 12:34:03 2023
root@debian-arm64:~#
root@debian-arm64:~# echo e > /proc/sysrq-trigger
Connection to 10.10.10.10 closed by remote host.
Connection to 10.10.10.10 closed.

Re: [PATCH] /proc/sysrq-trigger can now pause processing for one second

[Greg Kroah-Hartman]

On Mon, Dec 18, 2023 at 01:37:44PM +0100, Tomáš Mudruňka wrote:
> > What will kill it?  I feel like you are adding features to the kernel
> > that can be done in userspace, which is generally not a good idea.
> 
> The mere act of writing "e" to /proc/sysrq-trigger kills everything
> except for init, which is rather unfortunate when doing that through
> remote access, like ssh (or other). I can surely block SIGTERM in
> userspace by fixing all remote access software that exists to not exit
> after SIGTERM, but if i want to do SIGKILL and then execute few more
> sysrq actions (sync, unmount, reboot, ...) it surely is a problem
> unless i am doing this from init process. which sometimes is just not
> possible on remote system that have undergone some crash. and as linux
> admin with 13 years of experience i can safely say that situations
> with unresponsive init do happen every now and then. that is when i
> usually have to resort to rebooting the system remotely via
> sysrq-trigger. this process failing can be difference between me being
> able to fix issue remotely with minimum downtime and me having to
> physicaly visit datacenter during holidays.
> 
> BTW if still unclear, here is simple example of how running that
> suggested code will not work:
> 
> $ ssh root@10.10.10.10
> root@10.10.10.10's password:
> Last login: Wed Oct  4 12:34:03 2023
> root@debian-arm64:~#
> root@debian-arm64:~# echo e > /proc/sysrq-trigger
> Connection to 10.10.10.10 closed by remote host.
> Connection to 10.10.10.10 closed.

Great, then perhaps sysrq is not the thing you should be doing here?
Why is sysrq suddenly responsible for remote connection fixes?

I'm all for adding stuff that is useful, but really, sysrq is a "last
possible chance" type of thing, if you need it to reboot your box, your
box is hosed and it's not here to make it any less hosed.

Add pauses and soon you will want loops and then it's turing complete :)

Why not have a bpf script that does this instead?  :)

thanks,

greg k-h

Re: [PATCH] /proc/sysrq-trigger can now pause processing for one second

[Tomáš Mudruňka]

> Add pauses and soon you will want loops and then it's turing complete :)
>
> Why not have a bpf script that does this instead?  :)

Funny you mention this. For a moment i've actually thought someone
would come with this idea sooner or later. :-)
But i think we will all agree that there are several reasons why this
would be quite terrible idea...

Anyway for me the sysrq-trigger is about giving kernel linear
instructions on how to shutdown/reboot ASAP with minimal chance for
data corruption and least amount of userspace involvement possible. I
just want to kill everything more or less cleanly and safely reboot
without having to give control back to userspace script, because there
might be something wrong with userspace at the time.

Tom