Commit 649e9c70 authored by Oleg Nesterov's avatar Oleg Nesterov Committed by Steven Rostedt

tracing: Introduce trace_create_cpu_file() and tracing_get_cpu()

Every "file_operations" used by tracing_init_debugfs_percpu is buggy.
f_op->open/etc does:

	1. struct trace_cpu *tc = inode->i_private;
	   struct trace_array *tr = tc->tr;

	2. trace_array_get(tr) or fail;

	3. do_something(tc);

But tc (and tr) can be already freed before trace_array_get() is called.
And it doesn't matter whether this file is per-cpu or it was created by
init_tracer_debugfs(), free_percpu() or kfree() are equally bad.

Note that even 1. is not safe, the freed memory can be unmapped. But even
if it was safe trace_array_get() can wrongly succeed if we also race with
the next new_instance_create() which can re-allocate the same tr, or tc
was overwritten and ->tr points to the valid tr. In this case 3. uses the
freed/reused memory.

Add the new trivial helper, trace_create_cpu_file() which simply calls
trace_create_file() and encodes "cpu" in "struct inode". Another helper,
tracing_get_cpu() will be used to read cpu_nr-or-RING_BUFFER_ALL_CPUS.

The patch abuses ->i_cdev to encode the number, it is never used unless
the file is S_ISCHR(). But we could use something else, say, i_bytes or
even ->d_fsdata. In any case this hack is hidden inside these 2 helpers,
it would be trivial to change them if needed.

This patch only changes tracing_init_debugfs_percpu() to use the new
trace_create_cpu_file(), the next patches will change file_operations.

Note: tracing_get_cpu(inode) is always safe but you can't trust the
result unless trace_array_get() was called, without trace_types_lock
which acts as a barrier it can wrongly return RING_BUFFER_ALL_CPUS.

Link: http://lkml.kernel.org/r/20130723152554.GA23710@redhat.com

Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarSteven Rostedt <rostedt@goodmis.org>
parent e70e78e3
...@@ -2843,6 +2843,17 @@ static int s_show(struct seq_file *m, void *v) ...@@ -2843,6 +2843,17 @@ static int s_show(struct seq_file *m, void *v)
return 0; return 0;
} }
/*
* Should be used after trace_array_get(), trace_types_lock
* ensures that i_cdev was already initialized.
*/
static inline int tracing_get_cpu(struct inode *inode)
{
if (inode->i_cdev) /* See trace_create_cpu_file() */
return (long)inode->i_cdev - 1;
return RING_BUFFER_ALL_CPUS;
}
static const struct seq_operations tracer_seq_ops = { static const struct seq_operations tracer_seq_ops = {
.start = s_start, .start = s_start,
.next = s_next, .next = s_next,
...@@ -5529,6 +5540,17 @@ static struct dentry *tracing_dentry_percpu(struct trace_array *tr, int cpu) ...@@ -5529,6 +5540,17 @@ static struct dentry *tracing_dentry_percpu(struct trace_array *tr, int cpu)
return tr->percpu_dir; return tr->percpu_dir;
} }
static struct dentry *
trace_create_cpu_file(const char *name, umode_t mode, struct dentry *parent,
void *data, long cpu, const struct file_operations *fops)
{
struct dentry *ret = trace_create_file(name, mode, parent, data, fops);
if (ret) /* See tracing_get_cpu() */
ret->d_inode->i_cdev = (void *)(cpu + 1);
return ret;
}
static void static void
tracing_init_debugfs_percpu(struct trace_array *tr, long cpu) tracing_init_debugfs_percpu(struct trace_array *tr, long cpu)
{ {
...@@ -5548,28 +5570,28 @@ tracing_init_debugfs_percpu(struct trace_array *tr, long cpu) ...@@ -5548,28 +5570,28 @@ tracing_init_debugfs_percpu(struct trace_array *tr, long cpu)
} }
/* per cpu trace_pipe */ /* per cpu trace_pipe */
trace_create_file("trace_pipe", 0444, d_cpu, trace_create_cpu_file("trace_pipe", 0444, d_cpu,
(void *)&data->trace_cpu, &tracing_pipe_fops); &data->trace_cpu, cpu, &tracing_pipe_fops);
/* per cpu trace */ /* per cpu trace */
trace_create_file("trace", 0644, d_cpu, trace_create_cpu_file("trace", 0644, d_cpu,
(void *)&data->trace_cpu, &tracing_fops); &data->trace_cpu, cpu, &tracing_fops);
trace_create_file("trace_pipe_raw", 0444, d_cpu, trace_create_cpu_file("trace_pipe_raw", 0444, d_cpu,
(void *)&data->trace_cpu, &tracing_buffers_fops); &data->trace_cpu, cpu, &tracing_buffers_fops);
trace_create_file("stats", 0444, d_cpu, trace_create_cpu_file("stats", 0444, d_cpu,
(void *)&data->trace_cpu, &tracing_stats_fops); &data->trace_cpu, cpu, &tracing_stats_fops);
trace_create_file("buffer_size_kb", 0444, d_cpu, trace_create_cpu_file("buffer_size_kb", 0444, d_cpu,
(void *)&data->trace_cpu, &tracing_entries_fops); &data->trace_cpu, cpu, &tracing_entries_fops);
#ifdef CONFIG_TRACER_SNAPSHOT #ifdef CONFIG_TRACER_SNAPSHOT
trace_create_file("snapshot", 0644, d_cpu, trace_create_cpu_file("snapshot", 0644, d_cpu,
(void *)&data->trace_cpu, &snapshot_fops); &data->trace_cpu, cpu, &snapshot_fops);
trace_create_file("snapshot_raw", 0444, d_cpu, trace_create_cpu_file("snapshot_raw", 0444, d_cpu,
(void *)&data->trace_cpu, &snapshot_raw_fops); &data->trace_cpu, cpu, &snapshot_raw_fops);
#endif #endif
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment