diff options
| author | Steven Rostedt <srostedt@redhat.com> | 2011-07-05 14:32:51 -0400 |
|---|---|---|
| committer | Steven Rostedt <rostedt@goodmis.org> | 2011-07-07 11:22:29 -0400 |
| commit | 40ee4dffff061399eb9358e0c8fcfbaf8de4c8fe (patch) | |
| tree | 88b61ccc46994332d6f59cc1e6b2848fffc9a990 /kernel/trace | |
| parent | e9dbfae53eeb9fc3d4bb7da3df87fa9875f5da02 (diff) | |
tracing: Have "enable" file use refcounts like the "filter" file
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>
Diffstat (limited to 'kernel/trace')
| -rw-r--r-- | kernel/trace/trace_events.c | 31 |
1 files changed, 22 insertions, 9 deletions
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index ffc5b2884af1..3e2a7c91c548 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c | |||
| @@ -557,7 +557,7 @@ system_enable_read(struct file *filp, char __user *ubuf, size_t cnt, | |||
| 557 | loff_t *ppos) | 557 | loff_t *ppos) |
| 558 | { | 558 | { |
| 559 | const char set_to_char[4] = { '?', '0', '1', 'X' }; | 559 | const char set_to_char[4] = { '?', '0', '1', 'X' }; |
| 560 | const char *system = filp->private_data; | 560 | struct event_subsystem *system = filp->private_data; |
| 561 | struct ftrace_event_call *call; | 561 | struct ftrace_event_call *call; |
| 562 | char buf[2]; | 562 | char buf[2]; |
| 563 | int set = 0; | 563 | int set = 0; |
| @@ -568,7 +568,7 @@ system_enable_read(struct file *filp, char __user *ubuf, size_t cnt, | |||
| 568 | if (!call->name || !call->class || !call->class->reg) | 568 | if (!call->name || !call->class || !call->class->reg) |
| 569 | continue; | 569 | continue; |
| 570 | 570 | ||
| 571 | if (system && strcmp(call->class->system, system) != 0) | 571 | if (system && strcmp(call->class->system, system->name) != 0) |
| 572 | continue; | 572 | continue; |
| 573 | 573 | ||
| 574 | /* | 574 | /* |
| @@ -598,7 +598,8 @@ static ssize_t | |||
| 598 | system_enable_write(struct file *filp, const char __user *ubuf, size_t cnt, | 598 | system_enable_write(struct file *filp, const char __user *ubuf, size_t cnt, |
| 599 | loff_t *ppos) | 599 | loff_t *ppos) |
| 600 | { | 600 | { |
| 601 | const char *system = filp->private_data; | 601 | struct event_subsystem *system = filp->private_data; |
| 602 | const char *name = NULL; | ||
| 602 | unsigned long val; | 603 | unsigned long val; |
| 603 | char buf[64]; | 604 | char buf[64]; |
| 604 | ssize_t ret; | 605 | ssize_t ret; |
| @@ -622,7 +623,14 @@ system_enable_write(struct file *filp, const char __user *ubuf, size_t cnt, | |||
| 622 | if (val != 0 && val != 1) | 623 | if (val != 0 && val != 1) |
| 623 | return -EINVAL; | 624 | return -EINVAL; |
| 624 | 625 | ||
| 625 | ret = __ftrace_set_clr_event(NULL, system, NULL, val); | 626 | /* |
| 627 | * Opening of "enable" adds a ref count to system, | ||
| 628 | * so the name is safe to use. | ||
| 629 | */ | ||
| 630 | if (system) | ||
| 631 | name = system->name; | ||
| 632 | |||
| 633 | ret = __ftrace_set_clr_event(NULL, name, NULL, val); | ||
| 626 | if (ret) | 634 | if (ret) |
| 627 | goto out; | 635 | goto out; |
| 628 | 636 | ||
| @@ -862,6 +870,9 @@ static int subsystem_open(struct inode *inode, struct file *filp) | |||
| 862 | struct event_subsystem *system = NULL; | 870 | struct event_subsystem *system = NULL; |
| 863 | int ret; | 871 | int ret; |
| 864 | 872 | ||
| 873 | if (!inode->i_private) | ||
| 874 | goto skip_search; | ||
| 875 | |||
| 865 | /* Make sure the system still exists */ | 876 | /* Make sure the system still exists */ |
| 866 | mutex_lock(&event_mutex); | 877 | mutex_lock(&event_mutex); |
| 867 | list_for_each_entry(system, &event_subsystems, list) { | 878 | list_for_each_entry(system, &event_subsystems, list) { |
| @@ -880,8 +891,9 @@ static int subsystem_open(struct inode *inode, struct file *filp) | |||
| 880 | if (system != inode->i_private) | 891 | if (system != inode->i_private) |
| 881 | return -ENODEV; | 892 | return -ENODEV; |
| 882 | 893 | ||
| 894 | skip_search: | ||
| 883 | ret = tracing_open_generic(inode, filp); | 895 | ret = tracing_open_generic(inode, filp); |
| 884 | if (ret < 0) | 896 | if (ret < 0 && system) |
| 885 | put_system(system); | 897 | put_system(system); |
| 886 | 898 | ||
| 887 | return ret; | 899 | return ret; |
| @@ -891,7 +903,8 @@ static int subsystem_release(struct inode *inode, struct file *file) | |||
| 891 | { | 903 | { |
| 892 | struct event_subsystem *system = inode->i_private; | 904 | struct event_subsystem *system = inode->i_private; |
| 893 | 905 | ||
| 894 | put_system(system); | 906 | if (system) |
| 907 | put_system(system); | ||
| 895 | 908 | ||
| 896 | return 0; | 909 | return 0; |
| 897 | } | 910 | } |
| @@ -1041,10 +1054,11 @@ static const struct file_operations ftrace_subsystem_filter_fops = { | |||
| 1041 | }; | 1054 | }; |
| 1042 | 1055 | ||
| 1043 | static const struct file_operations ftrace_system_enable_fops = { | 1056 | static const struct file_operations ftrace_system_enable_fops = { |
| 1044 | .open = tracing_open_generic, | 1057 | .open = subsystem_open, |
| 1045 | .read = system_enable_read, | 1058 | .read = system_enable_read, |
| 1046 | .write = system_enable_write, | 1059 | .write = system_enable_write, |
| 1047 | .llseek = default_llseek, | 1060 | .llseek = default_llseek, |
| 1061 | .release = subsystem_release, | ||
| 1048 | }; | 1062 | }; |
| 1049 | 1063 | ||
| 1050 | static const struct file_operations ftrace_show_header_fops = { | 1064 | static const struct file_operations ftrace_show_header_fops = { |
| @@ -1133,8 +1147,7 @@ event_subsystem_dir(const char *name, struct dentry *d_events) | |||
| 1133 | "'%s/filter' entry\n", name); | 1147 | "'%s/filter' entry\n", name); |
| 1134 | } | 1148 | } |
| 1135 | 1149 | ||
| 1136 | trace_create_file("enable", 0644, system->entry, | 1150 | trace_create_file("enable", 0644, system->entry, system, |
| 1137 | (void *)system->name, | ||
| 1138 | &ftrace_system_enable_fops); | 1151 | &ftrace_system_enable_fops); |
| 1139 | 1152 | ||
| 1140 | return system->entry; | 1153 | return system->entry; |
