linux-kernel - Re: [PATCH] tracing: Add system trigger file to enable triggers for all the system's events

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: <64672c94fe19f0e87fbe9832eb26507ecaad50e9.camel@kernel.org>
Date: Mon, 01 Dec 2025 16:56:45 -0600
From: Tom Zanussi <zanussi@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>, LKML
 <linux-kernel@...r.kernel.org>,  Linux Trace Kernel
 <linux-trace-kernel@...r.kernel.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
	 <mathieu.desnoyers@...icios.com>
Subject: Re: [PATCH] tracing: Add system trigger file to enable triggers for
 all the system's events

Hi Steve,

On Wed, 2025-11-26 at 15:24 -0500, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@...dmis.org>
> 
> Currently a trigger can only be added to individual events. Some triggers
> (like stacktrace) can be useful to add as a bulk trigger for a set of
> system events (like interrupt or scheduling).
> 
> Add a trigger file to the system directories:
> 
>    /sys/kernel/tracing/events/*/trigger
> 
> And allow stacktrace trigger to be enabled for all those events.
> 
> Writing into the system/trigger file acts the same as writing into each of
> the system event's trigger files individually.
> 
> This also allows to remove a trigger from all events in a subsystem (even
> if it's not a subsystem trigger!).
> 

This looks very useful! Just a couple comments below..

> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
> Changes since v2: https://patch.msgid.link/20251125200837.31aae207@gandalf.local.home
> 
> - Removed unneeded NULL initialization of tr (Masami Hiramatsu)
> 
> 
> 

[ ... ]

> +static ssize_t
> +event_system_trigger_write(struct file *filp, const char __user *ubuf,
> +		    size_t cnt, loff_t *ppos)
> +{
> +	struct trace_subsystem_dir *dir = filp->private_data;
> +	struct event_command *p;
> +	char *command, *next;
> +	char *buf __free(kfree) = get_user_buf(ubuf, cnt);

nit: I think you could move this up a couple lines and keep the reverse
Christmas tree.

> +	bool remove = false;
> +	bool found = false;
> +	ssize_t ret;
> +
> +	if (!buf)
> +		return 0;
> +
> +	if (IS_ERR(buf))
> +		return PTR_ERR(buf);
> +
> +	/* system triggers are not allowed to have counters */
> +	if (strchr(buf, ':'))
> +		return -EINVAL;
> +

As mentioned by Masami, I think this would preclude the hist triggers
from having this enabled.

I'm guessing you didn't see the hist commands as one of the triggers
that would be useful as system triggers, but there might be a couple
cases where they could be if using the common fields e.g.

echo hist:keys=common_pid.execname:vals=hitcount > events/syscalls/trigger 

or anywhere just

echo hist:keys=common_stacktrace > trigger

Just about everything else would error out, but these might be worth having.

Maybe you could just change this check to look for :<number> at the end of
the command?

Thanks,

Tom