Skip to content

Commit

Permalink
tracing: Have "enable" file use refcounts like the "filter" file
Browse files Browse the repository at this point in the history
The "enable" file for the event system can be removed when a module
is unloaded and the event system only has events from that module.
As the event system nr_events count goes to zero, it may be freed
if its ref_count is also set to zero.

Like the "filter" file, the "enable" file may be opened by a task and
referenced later, after a module has been unloaded and the events for
that event system have been removed.

Although the "filter" file referenced the event system structure,
the "enable" file only references a pointer to the event system
name. Since the name is freed when the event system is removed,
it is possible that an access to the "enable" file may reference
a freed pointer.

Update the "enable" file to use the subsystem_open() routine that
the "filter" file uses, to keep a reference to the event system
structure while the "enable" file is opened.

Cc: <stable@kernel.org>
Reported-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
  • Loading branch information
Steven Rostedt authored and Steven Rostedt committed Jul 7, 2011
1 parent e9dbfae commit 40ee4df
Showing 1 changed file with 22 additions and 9 deletions.
31 changes: 22 additions & 9 deletions kernel/trace/trace_events.c
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ system_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
loff_t *ppos)
{
const char set_to_char[4] = { '?', '0', '1', 'X' };
const char *system = filp->private_data;
struct event_subsystem *system = filp->private_data;
struct ftrace_event_call *call;
char buf[2];
int set = 0;
Expand All @@ -568,7 +568,7 @@ system_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
if (!call->name || !call->class || !call->class->reg)
continue;

if (system && strcmp(call->class->system, system) != 0)
if (system && strcmp(call->class->system, system->name) != 0)
continue;

/*
Expand Down Expand Up @@ -598,7 +598,8 @@ static ssize_t
system_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
loff_t *ppos)
{
const char *system = filp->private_data;
struct event_subsystem *system = filp->private_data;
const char *name = NULL;
unsigned long val;
char buf[64];
ssize_t ret;
Expand All @@ -622,7 +623,14 @@ system_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
if (val != 0 && val != 1)
return -EINVAL;

ret = __ftrace_set_clr_event(NULL, system, NULL, val);
/*
* Opening of "enable" adds a ref count to system,
* so the name is safe to use.
*/
if (system)
name = system->name;

ret = __ftrace_set_clr_event(NULL, name, NULL, val);
if (ret)
goto out;

Expand Down Expand Up @@ -862,6 +870,9 @@ static int subsystem_open(struct inode *inode, struct file *filp)
struct event_subsystem *system = NULL;
int ret;

if (!inode->i_private)
goto skip_search;

/* Make sure the system still exists */
mutex_lock(&event_mutex);
list_for_each_entry(system, &event_subsystems, list) {
Expand All @@ -880,8 +891,9 @@ static int subsystem_open(struct inode *inode, struct file *filp)
if (system != inode->i_private)
return -ENODEV;

skip_search:
ret = tracing_open_generic(inode, filp);
if (ret < 0)
if (ret < 0 && system)
put_system(system);

return ret;
Expand All @@ -891,7 +903,8 @@ static int subsystem_release(struct inode *inode, struct file *file)
{
struct event_subsystem *system = inode->i_private;

put_system(system);
if (system)
put_system(system);

return 0;
}
Expand Down Expand Up @@ -1041,10 +1054,11 @@ static const struct file_operations ftrace_subsystem_filter_fops = {
};

static const struct file_operations ftrace_system_enable_fops = {
.open = tracing_open_generic,
.open = subsystem_open,
.read = system_enable_read,
.write = system_enable_write,
.llseek = default_llseek,
.release = subsystem_release,
};

static const struct file_operations ftrace_show_header_fops = {
Expand Down Expand Up @@ -1133,8 +1147,7 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
"'%s/filter' entry\n", name);
}

trace_create_file("enable", 0644, system->entry,
(void *)system->name,
trace_create_file("enable", 0644, system->entry, system,
&ftrace_system_enable_fops);

return system->entry;
Expand Down

0 comments on commit 40ee4df

Please sign in to comment.