]>
Commit | Line | Data |
---|---|---|
7cb15094 GKH |
1 | From e9dbfae53eeb9fc3d4bb7da3df87fa9875f5da02 Mon Sep 17 00:00:00 2001 |
2 | From: Steven Rostedt <srostedt@redhat.com> | |
3 | Date: Tue, 5 Jul 2011 11:36:06 -0400 | |
4 | Subject: tracing: Fix bug when reading system filters on module | |
5 | removal | |
6 | ||
7 | From: Steven Rostedt <srostedt@redhat.com> | |
8 | ||
9 | commit e9dbfae53eeb9fc3d4bb7da3df87fa9875f5da02 upstream. | |
10 | ||
11 | The event system is freed when its nr_events is set to zero. This happens | |
12 | when a module created an event system and then later the module is | |
13 | removed. Modules may share systems, so the system is allocated when | |
14 | it is created and freed when the modules are unloaded and all the | |
15 | events under the system are removed (nr_events set to zero). | |
16 | ||
17 | The problem arises when a task opened the "filter" file for the | |
18 | system. If the module is unloaded and it removed the last event for | |
19 | that system, the system structure is freed. If the task that opened | |
20 | the filter file accesses the "filter" file after the system has | |
21 | been freed, the system will access an invalid pointer. | |
22 | ||
23 | By adding a ref_count, and using it to keep track of what | |
24 | is using the event system, we can free it after all users | |
25 | are finished with the event system. | |
26 | ||
27 | Reported-by: Johannes Berg <johannes.berg@intel.com> | |
28 | Signed-off-by: Steven Rostedt <rostedt@goodmis.org> | |
29 | Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> | |
30 | ||
31 | --- | |
32 | kernel/trace/trace.h | 1 | |
33 | kernel/trace/trace_events.c | 86 ++++++++++++++++++++++++++++++++----- | |
34 | kernel/trace/trace_events_filter.c | 6 ++ | |
35 | 3 files changed, 82 insertions(+), 11 deletions(-) | |
36 | ||
37 | --- a/kernel/trace/trace.h | |
38 | +++ b/kernel/trace/trace.h | |
39 | @@ -677,6 +677,7 @@ struct event_subsystem { | |
40 | struct dentry *entry; | |
41 | struct event_filter *filter; | |
42 | int nr_events; | |
43 | + int ref_count; | |
44 | }; | |
45 | ||
46 | #define FILTER_PRED_INVALID ((unsigned short)-1) | |
47 | --- a/kernel/trace/trace_events.c | |
48 | +++ b/kernel/trace/trace_events.c | |
49 | @@ -244,6 +244,35 @@ static void ftrace_clear_events(void) | |
50 | mutex_unlock(&event_mutex); | |
51 | } | |
52 | ||
53 | +static void __put_system(struct event_subsystem *system) | |
54 | +{ | |
55 | + struct event_filter *filter = system->filter; | |
56 | + | |
57 | + WARN_ON_ONCE(system->ref_count == 0); | |
58 | + if (--system->ref_count) | |
59 | + return; | |
60 | + | |
61 | + if (filter) { | |
62 | + kfree(filter->filter_string); | |
63 | + kfree(filter); | |
64 | + } | |
65 | + kfree(system->name); | |
66 | + kfree(system); | |
67 | +} | |
68 | + | |
69 | +static void __get_system(struct event_subsystem *system) | |
70 | +{ | |
71 | + WARN_ON_ONCE(system->ref_count == 0); | |
72 | + system->ref_count++; | |
73 | +} | |
74 | + | |
75 | +static void put_system(struct event_subsystem *system) | |
76 | +{ | |
77 | + mutex_lock(&event_mutex); | |
78 | + __put_system(system); | |
79 | + mutex_unlock(&event_mutex); | |
80 | +} | |
81 | + | |
82 | /* | |
83 | * __ftrace_set_clr_event(NULL, NULL, NULL, set) will set/unset all events. | |
84 | */ | |
85 | @@ -826,6 +855,47 @@ event_filter_write(struct file *filp, co | |
86 | return cnt; | |
87 | } | |
88 | ||
89 | +static LIST_HEAD(event_subsystems); | |
90 | + | |
91 | +static int subsystem_open(struct inode *inode, struct file *filp) | |
92 | +{ | |
93 | + struct event_subsystem *system = NULL; | |
94 | + int ret; | |
95 | + | |
96 | + /* Make sure the system still exists */ | |
97 | + mutex_lock(&event_mutex); | |
98 | + list_for_each_entry(system, &event_subsystems, list) { | |
99 | + if (system == inode->i_private) { | |
100 | + /* Don't open systems with no events */ | |
101 | + if (!system->nr_events) { | |
102 | + system = NULL; | |
103 | + break; | |
104 | + } | |
105 | + __get_system(system); | |
106 | + break; | |
107 | + } | |
108 | + } | |
109 | + mutex_unlock(&event_mutex); | |
110 | + | |
111 | + if (system != inode->i_private) | |
112 | + return -ENODEV; | |
113 | + | |
114 | + ret = tracing_open_generic(inode, filp); | |
115 | + if (ret < 0) | |
116 | + put_system(system); | |
117 | + | |
118 | + return ret; | |
119 | +} | |
120 | + | |
121 | +static int subsystem_release(struct inode *inode, struct file *file) | |
122 | +{ | |
123 | + struct event_subsystem *system = inode->i_private; | |
124 | + | |
125 | + put_system(system); | |
126 | + | |
127 | + return 0; | |
128 | +} | |
129 | + | |
130 | static ssize_t | |
131 | subsystem_filter_read(struct file *filp, char __user *ubuf, size_t cnt, | |
132 | loff_t *ppos) | |
133 | @@ -963,10 +1033,11 @@ static const struct file_operations ftra | |
134 | }; | |
135 | ||
136 | static const struct file_operations ftrace_subsystem_filter_fops = { | |
137 | - .open = tracing_open_generic, | |
138 | + .open = subsystem_open, | |
139 | .read = subsystem_filter_read, | |
140 | .write = subsystem_filter_write, | |
141 | .llseek = default_llseek, | |
142 | + .release = subsystem_release, | |
143 | }; | |
144 | ||
145 | static const struct file_operations ftrace_system_enable_fops = { | |
146 | @@ -1002,8 +1073,6 @@ static struct dentry *event_trace_events | |
147 | return d_events; | |
148 | } | |
149 | ||
150 | -static LIST_HEAD(event_subsystems); | |
151 | - | |
152 | static struct dentry * | |
153 | event_subsystem_dir(const char *name, struct dentry *d_events) | |
154 | { | |
155 | @@ -1013,6 +1082,7 @@ event_subsystem_dir(const char *name, st | |
156 | /* First see if we did not already create this dir */ | |
157 | list_for_each_entry(system, &event_subsystems, list) { | |
158 | if (strcmp(system->name, name) == 0) { | |
159 | + __get_system(system); | |
160 | system->nr_events++; | |
161 | return system->entry; | |
162 | } | |
163 | @@ -1035,6 +1105,7 @@ event_subsystem_dir(const char *name, st | |
164 | } | |
165 | ||
166 | system->nr_events = 1; | |
167 | + system->ref_count = 1; | |
168 | system->name = kstrdup(name, GFP_KERNEL); | |
169 | if (!system->name) { | |
170 | debugfs_remove(system->entry); | |
171 | @@ -1184,16 +1255,9 @@ static void remove_subsystem_dir(const c | |
172 | list_for_each_entry(system, &event_subsystems, list) { | |
173 | if (strcmp(system->name, name) == 0) { | |
174 | if (!--system->nr_events) { | |
175 | - struct event_filter *filter = system->filter; | |
176 | - | |
177 | debugfs_remove_recursive(system->entry); | |
178 | list_del(&system->list); | |
179 | - if (filter) { | |
180 | - kfree(filter->filter_string); | |
181 | - kfree(filter); | |
182 | - } | |
183 | - kfree(system->name); | |
184 | - kfree(system); | |
185 | + __put_system(system); | |
186 | } | |
187 | break; | |
188 | } | |
189 | --- a/kernel/trace/trace_events_filter.c | |
190 | +++ b/kernel/trace/trace_events_filter.c | |
191 | @@ -1886,6 +1886,12 @@ int apply_subsystem_event_filter(struct | |
192 | ||
193 | mutex_lock(&event_mutex); | |
194 | ||
195 | + /* Make sure the system still has events */ | |
196 | + if (!system->nr_events) { | |
197 | + err = -ENODEV; | |
198 | + goto out_unlock; | |
199 | + } | |
200 | + | |
201 | if (!strcmp(strstrip(filter_string), "0")) { | |
202 | filter_free_subsystem_preds(system); | |
203 | remove_filter_string(system->filter); |