linux-kernel - Re: [PATCH] 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: <20250611063349.25187-1-marwanmhks@gmail.com>
Date: Wed, 11 Jun 2025 09:33:49 +0300
From: Marwan Seliem <marwanmhks@...il.com>
To: jirislaby@...nel.org
Cc: gregkh@...uxfoundation.org,
	akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org,
	linux-serial@...r.kernel.org
Subject: Re: [PATCH] tty: sysrq: Introduce compile-time crash-only mode

Hi Jiri,

Thank you for your review and feedback. Let me address your comments and provide more context about the use case for this change.

> I must admit I don't much understand the purpose of this. It can be
> spelled as: you can crash the system only by sysrq-c from now on. Don't
> use sysrq-r or others. Who did ask for this?

This change was created with embedded systems that have external subsystems in mind (like modems/co-processors) where we need:
    - The ability to trigger a full system crash (via sysrq-c) to collect subsystem crash dumps
    - While explicitly disabling all other sysrq functionality for security reasons

In these environments:
    - Crash dumps are essential for debugging
	- Other sysrq commands pose unnecessary security risks

> These inline #ifdefs are horrid.

Agreed. I will restructure this.

> This can be invoked from userspace. So you can nicely DoS the machine by
> the added warn, right? Hint: use ratelimiting.

Good point. I'll add ratelimiting to the pr_warn() calls or we can consider reducing these to pr_debug()?

> No need for this return ^^.

You're right, the second return is redundant. I'll clean this up.

> Is it for real?

>From a pure security viewpoint, expert advice is to remove this Magic Sysrq functionality, 
either with kernel.sysrq=0 in sysctl config file, or with a full kernel rebuild 
with n value for CONFIG_MAGIC_SYSRQ parameter.
This patch provides a middle ground that:
    1) Resolves the Core Security Conflict
		The CRASH_ONLY mode provides the minimal debug capability while eliminating:
            - Register dumps (disables 'p' command)
            - Filesystem operations (disables 'u'/sync commands)
            - All other privileged operations
    2) Security Architecture Benefits
	
		Traditional: All-or-nothing  
		│─────────────┬─────────────│  
			Full disable			Full enable  

		Our Approach: Principle of Least Privilege  
		│─────┬───────┬─────────────│  
			Off	Crash-only		Full enable  

For v2 of the patch, I'll make these improvements:
    1) Restructure to minimize #ifdef fiasco
	2) Add proper rate limiting on pr_warn/change it to pr_debug
	3) Clean up redundant return

Thank you again for your valuable feedback. I appreciate you taking the time to review this.

Warmest regards,
Marwan Seliem