linux-kernel - Re: [PATCH v2] tty: sysrq: Introduce compile-time crash-only mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2025071614-enlisted-railway-06b6@gregkh>
Date: Wed, 16 Jul 2025 10:05:22 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Marwan Seliem <marwanmhks@...il.com>
Cc: jirislaby@...nel.org, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH v2] tty: sysrq: Introduce compile-time crash-only mode

On Tue, Jul 08, 2025 at 10:57:01AM +0300, Marwan Seliem wrote:
> The Magic SysRq facility, while a powerful tool for debugging, presents a
> significant attack surface. A user with console access or sufficient
> privileges can use SysRq commands to reboot the system ('b'), terminate
> all processes ('i'), or perform other disruptive actions. These actions
> can lead to denial-of-service or be used to hide traces of an intrusion.
> 
> While SysRq can be disabled via a sysctl, a privileged user can often
> re-enable it at runtime. For hardened systems where the only required
> SysRq functionality is generating a kdump for post-mortem analysis, a
> stronger, permanent restriction is necessary.
> 
> This commit introduces the Kconfig option `CONFIG_MAGIC_SYSRQ_CRASH_ONLY`
> to provide a compile-time guarantee that only the 'c' (crash) command
> is available. This allows system administrators to build a kernel that
> supports critical crash dump generation while completely removing the
> attack surface presented by all other SysRq commands.
> 
> When `CONFIG_MAGIC_SYSRQ_CRASH_ONLY` is enabled, the kernel is hardened
> in the following ways:
> 
> 1.  Restricted Commands: Only the 'c' (trigger a system crash/dump)
>     SysRq command is operational. All other built-in SysRq commands are
>     disabled at compile time.
> 
> 2.  Runtime Registration Disabled: The kernel rejects any attempt to
>     register new SysRq key operations at runtime via `register_sysrq_key()`,
>     returning -EPERM.
> 
> 3.  Crash Command Unregistration Prevented: The 'c' (crash) command
>     cannot be unregistered.
> 
> 4.  Sysctl Hardening: The `/proc/sys/kernel/sysrq` interface is neutered.
>     Any write to this interface is rejected with -EPERM, preventing
>     runtime attempts to alter the SysRq mask. The kernel will only
>     permit the crash operation, regardless of the `sysrq_always_enabled`
>     kernel command line parameter.
> 
> 5.  Trigger Hardening: Writing any character other than 'c' to
>     `/proc/sysrq-trigger` is rejected with -EPERM.
> 
> 6.  Restricted Help Output: The help message, triggered by an invalid
>     SysRq key, will only list the 'c' (crash) command.
> 
> This feature provides a robust, compile-time mechanism to lock down
> SysRq functionality, ensuring that even a privileged user cannot bypass
> the intended security policy.
> 
> ---
> v2:
> - Adjust #ifdef style to align with existing patterns in sysrq.c.
> - Block writes to the /proc/sys/kernel/sysrq sysctl with -EPERM when
>   in crash-only mode, with a rate-limited warning.
> - Return -EPERM from the /proc/sysrq-trigger write handler if the
>   requested command is not 'c'.
> - Rate-limit warning messages generated from userspace-triggered events
>   to prevent log-flooding.
> 
> Affected files:
> - lib/Kconfig.debug: Added `CONFIG_MAGIC_SYSRQ_CRASH_ONLY`.
> - drivers/tty/sysrq.c: Implemented the conditional logic for
>   restricted mode.
> - kernel/sysctl.c: Use the sysrq_toggle_support return to deny illegal toggle
> 
> Signed-off-by: Marwan Seliem <marwanmhks@...il.com>
> ---
>  drivers/tty/sysrq.c | 87 +++++++++++++++++++++++++++++++++++++++++++--
>  kernel/sysctl.c     |  4 +--
>  lib/Kconfig.debug   | 27 ++++++++++++++
>  3 files changed, 113 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 6853c4660e7c..cccfdb0ed6d4 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -59,11 +59,25 @@
>  static int __read_mostly sysrq_enabled = CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE;
>  static bool __read_mostly sysrq_always_enabled;
>  
> +#ifdef CONFIG_MAGIC_SYSRQ_CRASH_ONLY
> +	/*
> +	* In CRASH_ONLY mode, sysrq is considered "on" only for the purpose
> +	* of allowing the crash command. The actual check for individual
> +	* commands happens in sysrq_on_mask().
> +	* For general "is sysrq on?" queries (like for input handler reg),
> +	* it should reflect that at least something (crash) is possible.
> +	*/
> +static bool sysrq_on(void)
> +{
> +	return true;
> +}
> +#else
>  static bool sysrq_on(void)
>  {
>  	return sysrq_enabled || sysrq_always_enabled;
>  }
>  
> +#endif

As stated before, I really don't like this new config option, and feel
that it does not do anything.  But becides this, you have implemented it
in a way that is almost unmaintainable, with these #ifdef in the .c
file.  That's not the proper kernel style at all, and will be a pain if
this would be accepted, so independent of the "is this a good idea or
not", we couldn't take this patch.

thanks,

greg k-h