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

Greg Kroah-Hartman
2023-11-13T21:37:41Z

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