public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sysrq: add optional logging of caller info on /proc/sysrq-trigger write
@ 2026-04-16 13:14 Xiang Gao
  2026-04-18  7:20 ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Xiang Gao @ 2026-04-16 13:14 UTC (permalink / raw)
  To: gregkh, jirislaby; +Cc: akpm, linux-kernel, linux-serial, Xiang Gao

From: Xiang Gao <gaoxiang17@xiaomi.com>

When /proc/sysrq-trigger is written to, there is no record of which
process triggered the sysrq operation. This makes it difficult to audit
or debug who initiated a sysrq action, especially when the write comes
from a shell spawned by system()/popen() where the immediate caller is
"sh" rather than the originating application.

Add CONFIG_MAGIC_SYSRQ_TRIGGER_LOG (default n) and a runtime toggle via
module parameter sysrq.trigger_log (default off). When both are enabled,
the kernel logs the triggering process's comm, pid, tgid, uid, and walks
up to 5 levels of the parent process chain. This allows tracing the
original initiator even through system()/popen()/fork+exec indirection.

Example output:
  sysrq: proc trigger: comm=sh pid=68 tgid=68 uid=0
  sysrq:   parent[0]: comm=my_app pid=67 tgid=67
  sysrq:   parent[1]: comm=init pid=1 tgid=1

Usage:
  # Compile-time: enable CONFIG_MAGIC_SYSRQ_TRIGGER_LOG=y
  # Runtime: echo 1 > /sys/module/sysrq/parameters/trigger_log
  # Or boot parameter: sysrq.trigger_log=1

Signed-off-by: Xiang Gao <gaoxiang17@xiaomi.com>
---
 drivers/tty/sysrq.c | 29 +++++++++++++++++++++++++++++
 lib/Kconfig.debug   | 16 ++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index c2e4b31b699a..e9277e7de35b 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -48,6 +48,9 @@
 #include <linux/uaccess.h>
 #include <linux/moduleparam.h>
 #include <linux/jiffies.h>
+#ifdef CONFIG_MAGIC_SYSRQ_TRIGGER_LOG
+#include <linux/cred.h>
+#endif
 #include <linux/syscalls.h>
 #include <linux/of.h>
 #include <linux/rcupdate.h>
@@ -59,6 +62,12 @@
 static int __read_mostly sysrq_enabled = CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE;
 static bool __read_mostly sysrq_always_enabled;
 
+#ifdef CONFIG_MAGIC_SYSRQ_TRIGGER_LOG
+static bool sysrq_trigger_log;
+module_param_named(trigger_log, sysrq_trigger_log, bool, 0644);
+MODULE_PARM_DESC(trigger_log, "Log caller info on /proc/sysrq-trigger write");
+#endif
+
 static bool sysrq_on(void)
 {
 	return sysrq_enabled || sysrq_always_enabled;
@@ -1209,6 +1218,26 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
 	bool bulk = false;
 	size_t i;
 
+#ifdef CONFIG_MAGIC_SYSRQ_TRIGGER_LOG
+	if (sysrq_trigger_log) {
+		struct task_struct *task;
+		int depth = 0;
+
+		pr_info("proc trigger: comm=%s pid=%d tgid=%d uid=%u\n",
+			current->comm, current->pid, current->tgid,
+			from_kuid(&init_user_ns, current_uid()));
+
+		rcu_read_lock();
+		task = current;
+		while (task->pid > 1 && depth < 5) {
+			task = rcu_dereference(task->real_parent);
+			pr_info("  parent[%d]: comm=%s pid=%d tgid=%d\n",
+				depth++, task->comm, task->pid, task->tgid);
+		}
+		rcu_read_unlock();
+	}
+#endif
+
 	for (i = 0; i < count; i++) {
 		char c;
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index aac60b6cfa4b..46bd361decd0 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -705,6 +705,22 @@ config MAGIC_SYSRQ_SERIAL_SEQUENCE
 
 	  If unsure, leave an empty string and the option will not be enabled.
 
+config MAGIC_SYSRQ_TRIGGER_LOG
+	bool "Log caller info on /proc/sysrq-trigger write"
+	depends on MAGIC_SYSRQ
+	default n
+	help
+	  If you say Y here, the kernel can log the process name, pid,
+	  tgid, uid and parent process chain when /proc/sysrq-trigger
+	  is written to. This is useful for auditing who triggered a
+	  sysrq operation.
+
+	  The logging is controlled at runtime via module parameter
+	  sysrq.trigger_log (default off). Enable it with:
+	    echo 1 > /sys/module/sysrq/parameters/trigger_log
+
+	  If unsure, say N.
+
 config DEBUG_FS
 	bool "Debug Filesystem"
 	help
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] sysrq: add optional logging of caller info on /proc/sysrq-trigger write
  2026-04-16 13:14 [PATCH] sysrq: add optional logging of caller info on /proc/sysrq-trigger write Xiang Gao
@ 2026-04-18  7:20 ` Greg KH
       [not found]   ` <e973ea4e812b4aef95bce54732c406d7@xiaomi.com>
  2026-04-20 12:03   ` Xiang Gao
  0 siblings, 2 replies; 4+ messages in thread
From: Greg KH @ 2026-04-18  7:20 UTC (permalink / raw)
  To: Xiang Gao; +Cc: jirislaby, akpm, linux-kernel, linux-serial, Xiang Gao

On Thu, Apr 16, 2026 at 09:14:19PM +0800, Xiang Gao wrote:
> From: Xiang Gao <gaoxiang17@xiaomi.com>
> 
> When /proc/sysrq-trigger is written to, there is no record of which
> process triggered the sysrq operation. This makes it difficult to audit
> or debug who initiated a sysrq action, especially when the write comes
> from a shell spawned by system()/popen() where the immediate caller is
> "sh" rather than the originating application.
> 
> Add CONFIG_MAGIC_SYSRQ_TRIGGER_LOG (default n) and a runtime toggle via
> module parameter sysrq.trigger_log (default off). When both are enabled,
> the kernel logs the triggering process's comm, pid, tgid, uid, and walks
> up to 5 levels of the parent process chain. This allows tracing the
> original initiator even through system()/popen()/fork+exec indirection.
> 
> Example output:
>   sysrq: proc trigger: comm=sh pid=68 tgid=68 uid=0
>   sysrq:   parent[0]: comm=my_app pid=67 tgid=67
>   sysrq:   parent[1]: comm=init pid=1 tgid=1
> 
> Usage:
>   # Compile-time: enable CONFIG_MAGIC_SYSRQ_TRIGGER_LOG=y
>   # Runtime: echo 1 > /sys/module/sysrq/parameters/trigger_log
>   # Or boot parameter: sysrq.trigger_log=1
> 
> Signed-off-by: Xiang Gao <gaoxiang17@xiaomi.com>
> ---
>  drivers/tty/sysrq.c | 29 +++++++++++++++++++++++++++++
>  lib/Kconfig.debug   | 16 ++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index c2e4b31b699a..e9277e7de35b 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -48,6 +48,9 @@
>  #include <linux/uaccess.h>
>  #include <linux/moduleparam.h>
>  #include <linux/jiffies.h>
> +#ifdef CONFIG_MAGIC_SYSRQ_TRIGGER_LOG
> +#include <linux/cred.h>
> +#endif

We really do not want or like #ifdef in .c files, and for stuff like
this, it is not needed at all.

>  #include <linux/syscalls.h>
>  #include <linux/of.h>
>  #include <linux/rcupdate.h>
> @@ -59,6 +62,12 @@
>  static int __read_mostly sysrq_enabled = CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE;
>  static bool __read_mostly sysrq_always_enabled;
>  
> +#ifdef CONFIG_MAGIC_SYSRQ_TRIGGER_LOG
> +static bool sysrq_trigger_log;
> +module_param_named(trigger_log, sysrq_trigger_log, bool, 0644);
> +MODULE_PARM_DESC(trigger_log, "Log caller info on /proc/sysrq-trigger write");
> +#endif

Module parameters are really not the way for stuff like this.  And why
would such a "tiny" option need a config option at all?  If you don't
use/need it, it's only a single bool being used?


> +
>  static bool sysrq_on(void)
>  {
>  	return sysrq_enabled || sysrq_always_enabled;
> @@ -1209,6 +1218,26 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
>  	bool bulk = false;
>  	size_t i;
>  
> +#ifdef CONFIG_MAGIC_SYSRQ_TRIGGER_LOG
> +	if (sysrq_trigger_log) {
> +		struct task_struct *task;
> +		int depth = 0;
> +
> +		pr_info("proc trigger: comm=%s pid=%d tgid=%d uid=%u\n",
> +			current->comm, current->pid, current->tgid,
> +			from_kuid(&init_user_ns, current_uid()));

The kernel log is not there for doing audits and the like, so is this
just a debug option?  

> +
> +		rcu_read_lock();
> +		task = current;
> +		while (task->pid > 1 && depth < 5) {
> +			task = rcu_dereference(task->real_parent);
> +			pr_info("  parent[%d]: comm=%s pid=%d tgid=%d\n",
> +				depth++, task->comm, task->pid, task->tgid);
> +		}
> +		rcu_read_unlock();

This might cause problems for when the system is hung and sysrq is the
only way to reboot the box.  Have you tried it in that situation?



> +	}
> +#endif
> +
>  	for (i = 0; i < count; i++) {
>  		char c;
>  
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index aac60b6cfa4b..46bd361decd0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -705,6 +705,22 @@ config MAGIC_SYSRQ_SERIAL_SEQUENCE
>  
>  	  If unsure, leave an empty string and the option will not be enabled.
>  
> +config MAGIC_SYSRQ_TRIGGER_LOG
> +	bool "Log caller info on /proc/sysrq-trigger write"
> +	depends on MAGIC_SYSRQ
> +	default n

n is always the default, no need to add it again.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: 答复: [External Mail]Re: [PATCH] sysrq: add optional logging of caller info on /proc/sysrq-trigger write
       [not found]   ` <e973ea4e812b4aef95bce54732c406d7@xiaomi.com>
@ 2026-04-20  8:05     ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2026-04-20  8:05 UTC (permalink / raw)
  To: 高翔
  Cc: Xiang Gao, jirislaby@kernel.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org

On Mon, Apr 20, 2026 at 07:32:45AM +0000, 高翔 wrote:

<snip>

you sent this in html format, which was rejected by the public lists :(

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sysrq: add optional logging of caller info on /proc/sysrq-trigger write
  2026-04-18  7:20 ` Greg KH
       [not found]   ` <e973ea4e812b4aef95bce54732c406d7@xiaomi.com>
@ 2026-04-20 12:03   ` Xiang Gao
  1 sibling, 0 replies; 4+ messages in thread
From: Xiang Gao @ 2026-04-20 12:03 UTC (permalink / raw)
  To: gregkh; +Cc: jirislaby, akpm, linux-kernel, linux-serial, gaoxiang17,
	Xiang Gao

On Sat, Apr 18, 2026 at 09:20:14AM +0200, Greg Kroah-Hartman wrote:
> We really do not want or like #ifdef in .c files, and for stuff like
> this, it is not needed at all.
[...]
> Module parameters are really not the way for stuff like this.  And why
> would such a "tiny" option need a config option at all?  If you don't
> use/need it, it's only a single bool being used?
[...]
> n is always the default, no need to add it again.

Agreed on all of these. The #ifdef, the module_param, the Kconfig
entry and the 'default n' all go away -- the feature will be
unconditional.

Before respinning I would like to align on the approach, because
your comments on the kernel log and the ancestry walk touch the
core of what this patch should actually be. Let me describe the
real use case first, since I do not think I made it clear in v1.

Use case

  - Userspace triggers are typically several processes deep:
    some service -> library -> system("echo c > /proc/sysrq-trigger")
    -> sh. The immediate writer is "sh", the actually responsible
    component is several levels up.
  - The sysrq character we care about is almost always 'c', 'b' or
    'o' -- the machine panics, reboots, or powers off as a direct
    result of the write.
  - *After* the device has come back up, we need to identify which
    userspace component issued the write. Otherwise there is no way
    to debug why devices in the field are rebooting themselves.

> The kernel log is not there for doing audits and the like, so is this
> just a debug option?

I agree that security audit belongs in the audit subsystem, not in
printk. What this patch is trying to do is post-mortem diagnostic
for crash/reboot triage, and audit_log() cannot deliver on that:

  1. audit_log() is asynchronous -- the record is queued, then
     delivered via kauditd -> netlink to a userspace subscriber.
     For sysrq c/b/o the panic/reboot happens before kauditd gets
     to run, so the record is never delivered.

  2. In our deployment environment there is no userspace auditd
     persisting records to disk, and audit records are not
     preserved across reboot. The kernel ring buffer, via pstore /
     ramoops, is the only channel whose contents survive the
     reboot and are readable on the next boot. pr_info() is the
     only mechanism that delivers into that persistent path.

I would frame this not as audit but as post-mortem diagnostic,
retitle the commit accordingly ("sysrq: log triggering task info
for post-mortem diagnostics") and drop the audit framing from the
changelog entirely.

> This might cause problems for when the system is hung and sysrq is the
> only way to reboot the box.  Have you tried it in that situation?

Fair -- the while-loop walking 5 levels has to go. The reason it
was there in v1 is the "sh from system()/popen()" case above: the
writer's comm is "sh", and the information we actually want is one
or more levels up the parent chain.

On the hung-system concern specifically: /proc/sysrq-trigger only
runs when userspace is alive enough to complete a write(), so the
keyboard sysrq path via handle_sysrq() is not affected by this
change -- my patch only touches write_sysrq_trigger(). Within that
function, rcu_read_lock() / rcu_dereference() take no locks,
allocate nothing, and are bounded-constant work, so on paper they
should not introduce a new hang. I have not yet reproduced a
partially-hung system to verify this empirically; I will do so
before v2 and report back. Please correct me if I am misreading
the concern.

That said, two options I can see, would like your steer before I
respin:

  (a) One bounded rcu_dereference() of current->real_parent.
      Logs current's pid/comm plus ppid + parent comm. One
      level, no loop. Covers the direct case
        app -> system("echo c > /proc/sysrq-trigger") -> sh
      where sh's parent is the app.

  (b) Two unrolled rcu_dereference()s -- current->real_parent
      and that task's real_parent. Still no loop, just two
      fixed dereferences. Covers the common indirect case
        service -> library -> system(...) -> sh
      where sh's parent is the thin library wrapper and the
      grandparent is the originating service, which is what we
      actually need to identify in the field.

In both cases the RCU read is a fixed number of dereferences,
not iteration over a data structure, so the total work is
bounded and constant.

I lean toward (b): one level is not enough to reach the
originating service in the indirect pattern this patch was
written for. If two dereferences in the sysrq path are still
too much, I will fall back to (a).

Which direction would you accept? I will respin based on your
answer.

Thanks,
Xiang

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-20 12:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-16 13:14 [PATCH] sysrq: add optional logging of caller info on /proc/sysrq-trigger write Xiang Gao
2026-04-18  7:20 ` Greg KH
     [not found]   ` <e973ea4e812b4aef95bce54732c406d7@xiaomi.com>
2026-04-20  8:05     ` 答复: [External Mail]Re: " Greg KH
2026-04-20 12:03   ` Xiang Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox