| 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
| ||
|
Message-ID: <ZW9oNUanjo2qn7r_@google.com>
Date: Tue, 5 Dec 2023 19:13:09 +0100
From: Dmytro Maluka <dmaluka@...omium.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Sean Paul <seanpaul@...omium.org>,
Arun Easi <aeasi@...vell.com>, Daniel Wagner <dwagner@...e.de>
Subject: Re: [PATCH] tracing: Allow creating instances with specified system
events
On Tue, Nov 28, 2023 at 12:21:17PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@...dmis.org>
>
> A trace instance may only need to enable specific events. As the eventfs
> directory of an instance currently creates all events which adds overhead,
> allow internal instances to be created with just the events in systems
> that they care about. This currently only deals with systems and not
> individual events, but this should bring down the overhead of creating
> instances for specific use cases quite bit.
>
> The trace_array_get_by_name() now has another parameter "systems". This
> parameter is a const string pointer of a comma/space separated list of
> event systems that should be created by the trace_array. (Note if the
> trace_array already exists, this parameter is ignored).
>
> The list of systems is saved and if a module is loaded, its events will
> not be added unless the system for those events also match the systems
> string.
>
> Note that all dynamic events are still added as they are created by the
> user.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
Tested-by: Dmytro Maluka <dmaluka@...omium.org>
> Changes since RFC: https://lore.kernel.org/all/20231127174108.3c331c9c@gandalf.local.home/
>
> - Record "systems" in the trace_array so that only events with the system
> name will be added. This includes modules that create events after the
> trace_array is created.
>
> - Dynamic events had to be specified directly to still allow them to be
> created.
>
> drivers/scsi/qla2xxx/qla_os.c | 2 +-
> include/linux/trace.h | 4 +--
> include/linux/trace_events.h | 4 +++
> kernel/trace/trace.c | 23 ++++++++++---
> kernel/trace/trace.h | 1 +
> kernel/trace/trace_boot.c | 2 +-
> kernel/trace/trace_events.c | 52 +++++++++++++++++++++++++++--
> samples/ftrace/sample-trace-array.c | 2 +-
> 8 files changed, 78 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 03348f605c2e..dd674378f2f3 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -2889,7 +2889,7 @@ static void qla2x00_iocb_work_fn(struct work_struct *work)
> static void
> qla_trace_init(void)
> {
> - qla_trc_array = trace_array_get_by_name("qla2xxx");
> + qla_trc_array = trace_array_get_by_name("qla2xxx", NULL);
> if (!qla_trc_array) {
> ql_log(ql_log_fatal, NULL, 0x0001,
> "Unable to create qla2xxx trace instance, instance logging will be disabled.\n");
> diff --git a/include/linux/trace.h b/include/linux/trace.h
> index 2a70a447184c..fdcd76b7be83 100644
> --- a/include/linux/trace.h
> +++ b/include/linux/trace.h
> @@ -51,7 +51,7 @@ int trace_array_printk(struct trace_array *tr, unsigned long ip,
> const char *fmt, ...);
> int trace_array_init_printk(struct trace_array *tr);
> void trace_array_put(struct trace_array *tr);
> -struct trace_array *trace_array_get_by_name(const char *name);
> +struct trace_array *trace_array_get_by_name(const char *name, const char *systems);
> int trace_array_destroy(struct trace_array *tr);
>
> /* For osnoise tracer */
> @@ -84,7 +84,7 @@ static inline int trace_array_init_printk(struct trace_array *tr)
> static inline void trace_array_put(struct trace_array *tr)
> {
> }
> -static inline struct trace_array *trace_array_get_by_name(const char *name)
> +static inline struct trace_array *trace_array_get_by_name(const char *name, const char *systems)
> {
> return NULL;
> }
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index d68ff9b1247f..ff818cab07b8 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -365,6 +365,10 @@ enum {
>
> #define TRACE_EVENT_FL_UKPROBE (TRACE_EVENT_FL_KPROBE | TRACE_EVENT_FL_UPROBE)
>
> +#define TRACE_EVENT_FL_DYNAMIC_MASK \
> + (TRACE_EVENT_FL_DYNAMIC | TRACE_EVENT_FL_KPROBE | \
> + TRACE_EVENT_FL_UPROBE | TRACE_EVENT_FL_EPROBE | TRACE_EVENT_FL_FPROBE)
> +
> struct trace_event_call {
> struct list_head list;
> struct trace_event_class *class;
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 9aebf904ff97..1c7129d07bfb 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -9520,7 +9520,8 @@ static int trace_array_create_dir(struct trace_array *tr)
> return ret;
> }
>
> -static struct trace_array *trace_array_create(const char *name)
> +static struct trace_array *
> +trace_array_create_systems(const char *name, const char *systems)
> {
> struct trace_array *tr;
> int ret;
> @@ -9540,6 +9541,12 @@ static struct trace_array *trace_array_create(const char *name)
> if (!zalloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
> goto out_free_tr;
>
> + if (systems) {
> + tr->system_names = kstrdup_const(systems, GFP_KERNEL);
> + if (!tr->system_names)
> + goto out_free_tr;
> + }
> +
> tr->trace_flags = global_trace.trace_flags & ~ZEROED_TRACE_FLAGS;
>
> cpumask_copy(tr->tracing_cpumask, cpu_all_mask);
> @@ -9586,12 +9593,18 @@ static struct trace_array *trace_array_create(const char *name)
> free_trace_buffers(tr);
> free_cpumask_var(tr->pipe_cpumask);
> free_cpumask_var(tr->tracing_cpumask);
> + kfree_const(tr->system_names);
> kfree(tr->name);
> kfree(tr);
>
> return ERR_PTR(ret);
> }
>
> +static struct trace_array *trace_array_create(const char *name)
> +{
> + return trace_array_create_systems(name, NULL);
> +}
> +
> static int instance_mkdir(const char *name)
> {
> struct trace_array *tr;
> @@ -9617,6 +9630,7 @@ static int instance_mkdir(const char *name)
> /**
> * trace_array_get_by_name - Create/Lookup a trace array, given its name.
> * @name: The name of the trace array to be looked up/created.
> + * @system: A list of systems to create event directories for (NULL for all)
> *
> * Returns pointer to trace array with given name.
> * NULL, if it cannot be created.
> @@ -9630,7 +9644,7 @@ static int instance_mkdir(const char *name)
> * trace_array_put() is called, user space can not delete it.
> *
> */
> -struct trace_array *trace_array_get_by_name(const char *name)
> +struct trace_array *trace_array_get_by_name(const char *name, const char *systems)
> {
> struct trace_array *tr;
>
> @@ -9642,7 +9656,7 @@ struct trace_array *trace_array_get_by_name(const char *name)
> goto out_unlock;
> }
>
> - tr = trace_array_create(name);
> + tr = trace_array_create_systems(name, systems);
>
> if (IS_ERR(tr))
> tr = NULL;
> @@ -9689,6 +9703,7 @@ static int __remove_instance(struct trace_array *tr)
>
> free_cpumask_var(tr->pipe_cpumask);
> free_cpumask_var(tr->tracing_cpumask);
> + kfree_const(tr->system_names);
> kfree(tr->name);
> kfree(tr);
>
> @@ -10407,7 +10422,7 @@ __init static void enable_instances(void)
> if (IS_ENABLED(CONFIG_TRACER_MAX_TRACE))
> do_allocate_snapshot(tok);
>
> - tr = trace_array_get_by_name(tok);
> + tr = trace_array_get_by_name(tok, NULL);
> if (!tr) {
> pr_warn("Failed to create instance buffer %s\n", curr_str);
> continue;
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index b7f4ea25a194..97b01dfd7070 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -377,6 +377,7 @@ struct trace_array {
> unsigned char trace_flags_index[TRACE_FLAGS_MAX_SIZE];
> unsigned int flags;
> raw_spinlock_t start_lock;
> + const char *system_names;
> struct list_head err_log;
> struct dentry *dir;
> struct dentry *options;
> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
> index 7ccc7a8e155b..dbe29b4c6a7a 100644
> --- a/kernel/trace/trace_boot.c
> +++ b/kernel/trace/trace_boot.c
> @@ -633,7 +633,7 @@ trace_boot_init_instances(struct xbc_node *node)
> if (!p || *p == '\0')
> continue;
>
> - tr = trace_array_get_by_name(p);
> + tr = trace_array_get_by_name(p, NULL);
> if (!tr) {
> pr_err("Failed to get trace instance %s\n", p);
> continue;
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index f29e815ca5b2..e8a56cf52cc1 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -2896,6 +2896,31 @@ void trace_event_eval_update(struct trace_eval_map **map, int len)
> up_write(&trace_event_sem);
> }
>
> +static bool event_in_systems(struct trace_event_call *call,
> + const char *systems)
> +{
> + const char *system;
> + const char *p;
> +
> + if (!systems)
> + return true;
> +
> + /* Dynamic events are always created */
> + if (call->flags & TRACE_EVENT_FL_DYNAMIC_MASK)
> + return true;
> +
> + system = call->class->system;
> + p = strstr(systems, system);
> + if (!p)
> + return false;
> +
> + if (p != systems && !isspace(*(p - 1)) && *(p - 1) != ',')
> + return false;
> +
> + p += strlen(system);
> + return !*p || isspace(*p) || *p == ',';
> +}
> +
> static struct trace_event_file *
> trace_create_new_event(struct trace_event_call *call,
> struct trace_array *tr)
> @@ -2905,9 +2930,12 @@ trace_create_new_event(struct trace_event_call *call,
> struct trace_event_file *file;
> unsigned int first;
>
> + if (!event_in_systems(call, tr->system_names))
> + return NULL;
> +
> file = kmem_cache_alloc(file_cachep, GFP_TRACE);
> if (!file)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
>
> pid_list = rcu_dereference_protected(tr->filtered_pids,
> lockdep_is_held(&event_mutex));
> @@ -2972,8 +3000,17 @@ __trace_add_new_event(struct trace_event_call *call, struct trace_array *tr)
> struct trace_event_file *file;
>
> file = trace_create_new_event(call, tr);
> + /*
> + * trace_create_new_event() returns ERR_PTR(-ENOMEM) if failed
> + * allocation, or NULL if the event is not part of the tr->system_names.
> + * When the event is not part of the tr->system_names, return zero, not
> + * an error.
> + */
> if (!file)
> - return -ENOMEM;
> + return 0;
> +
> + if (IS_ERR(file))
> + return PTR_ERR(file);
>
> if (eventdir_initialized)
> return event_create_dir(tr->event_dir, file);
> @@ -3012,8 +3049,17 @@ __trace_early_add_new_event(struct trace_event_call *call,
> int ret;
>
> file = trace_create_new_event(call, tr);
> + /*
> + * trace_create_new_event() returns ERR_PTR(-ENOMEM) if failed
> + * allocation, or NULL if the event is not part of the tr->system_names.
> + * When the event is not part of the tr->system_names, return zero, not
> + * an error.
> + */
> if (!file)
> - return -ENOMEM;
> + return 0;
> +
> + if (IS_ERR(file))
> + return PTR_ERR(file);
>
> ret = event_define_fields(call);
> if (ret)
> diff --git a/samples/ftrace/sample-trace-array.c b/samples/ftrace/sample-trace-array.c
> index 6aba02a31c96..89b72957e687 100644
> --- a/samples/ftrace/sample-trace-array.c
> +++ b/samples/ftrace/sample-trace-array.c
> @@ -105,7 +105,7 @@ static int __init sample_trace_array_init(void)
> * NOTE: This function increments the reference counter
> * associated with the trace array - "tr".
> */
> - tr = trace_array_get_by_name("sample-instance");
> + tr = trace_array_get_by_name("sample-instance", "sched,timer");
>
> if (!tr)
> return -1;
> --
> 2.42.0
>
Powered by blists - more mailing lists