History

tracing: Fix bug when reading system filters on module removal

The event system is freed when its nr_events is set to zero. This happens when a module created an event system and then later the module is removed. Modules may share systems, so the system is allocated when it is created and freed when the modules are unloaded and all the events under the system are removed (nr_events set to zero). The problem arises when a task opened the "filter" file for the system. If the module is unloaded and it removed the last event for that system, the system structure is freed. If the task that opened the filter file accesses the "filter" file after the system has been freed, the system will access an invalid pointer. By adding a ref_count, and using it to keep track of what is using the event system, we can free it after all users are finished with the event system.

Steven Rostedt committed 15 years ago

Commit e9dbfae1 parent 140fe3b3 files · +82 −11Browse files

Unified · Split

3 files
  1. kernel/trace/trace.h +1 −0
  2. kernel/trace/trace_events.c +75 −11
  3. kernel/trace/trace_events_filter.c +6 −0
kernel/trace/trace.h+1−0
@@ -677,6 +677,7 @@ struct event_subsystem { · 676 unmodified lines
677 struct dentry *entry;677 struct dentry *entry;
678 struct event_filter *filter;678 struct event_filter *filter;
679 int nr_events;679 int nr_events;
680 int ref_count;
680};681};
681 682
682#define FILTER_PRED_INVALID ((unsigned short)-1)683#define FILTER_PRED_INVALID ((unsigned short)-1)
kernel/trace/trace_events.c+75−11
@@ -244,6 +244,35 @@ static void ftrace_clear_events(void) · 243 unmodified lines
244 mutex_unlock(&event_mutex);244 mutex_unlock(&event_mutex);
245}245}
246 246
247static void __put_system(struct event_subsystem *system)
248{
249 struct event_filter *filter = system->filter;
250
251 WARN_ON_ONCE(system->ref_count == 0);
252 if (--system->ref_count)
253 return;
254
255 if (filter) {
256 kfree(filter->filter_string);
257 kfree(filter);
258 }
259 kfree(system->name);
260 kfree(system);
261}
262
263static void __get_system(struct event_subsystem *system)
264{
265 WARN_ON_ONCE(system->ref_count == 0);
266 system->ref_count++;
267}
268
269static void put_system(struct event_subsystem *system)
270{
271 mutex_lock(&event_mutex);
272 __put_system(system);
273 mutex_unlock(&event_mutex);
274}
275
247/*276/*
248 * __ftrace_set_clr_event(NULL, NULL, NULL, set) will set/unset all events.277 * __ftrace_set_clr_event(NULL, NULL, NULL, set) will set/unset all events.
249 */278 */
@@ -826,6 +855,47 @@ event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt, · 576 unmodified lines
826 return cnt;855 return cnt;
827}856}
828 857
858static LIST_HEAD(event_subsystems);
859
860static int subsystem_open(struct inode *inode, struct file *filp)
861{
862 struct event_subsystem *system = NULL;
863 int ret;
864
865 /* Make sure the system still exists */
866 mutex_lock(&event_mutex);
867 list_for_each_entry(system, &event_subsystems, list) {
868 if (system == inode->i_private) {
869 /* Don't open systems with no events */
870 if (!system->nr_events) {
871 system = NULL;
872 break;
873 }
874 __get_system(system);
875 break;
876 }
877 }
878 mutex_unlock(&event_mutex);
879
880 if (system != inode->i_private)
881 return -ENODEV;
882
883 ret = tracing_open_generic(inode, filp);
884 if (ret < 0)
885 put_system(system);
886
887 return ret;
888}
889
890static int subsystem_release(struct inode *inode, struct file *file)
891{
892 struct event_subsystem *system = inode->i_private;
893
894 put_system(system);
895
896 return 0;
897}
898
829static ssize_t899static ssize_t
830subsystem_filter_read(struct file *filp, char __user *ubuf, size_t cnt,900subsystem_filter_read(struct file *filp, char __user *ubuf, size_t cnt,
831 loff_t *ppos)901 loff_t *ppos)
@@ -963,10 +1033,11 @@ static const struct file_operations ftrace_event_filter_fops = { · 131 unmodified lines
963};1033};
964 1034
965static const struct file_operations ftrace_subsystem_filter_fops = {1035static const struct file_operations ftrace_subsystem_filter_fops = {
966 .open = tracing_open_generic,1036 .open = subsystem_open,
967 .read = subsystem_filter_read,1037 .read = subsystem_filter_read,
968 .write = subsystem_filter_write,1038 .write = subsystem_filter_write,
969 .llseek = default_llseek,1039 .llseek = default_llseek,
1040 .release = subsystem_release,
970};1041};
971 1042
972static const struct file_operations ftrace_system_enable_fops = {1043static const struct file_operations ftrace_system_enable_fops = {
@@ -1002,8 +1073,6 @@ static struct dentry *event_trace_events_dir(void) · 29 unmodified lines
1002 return d_events;1073 return d_events;
1003}1074}
1004 1075
1005static LIST_HEAD(event_subsystems);
1006
1007static struct dentry *1076static struct dentry *
1008event_subsystem_dir(const char *name, struct dentry *d_events)1077event_subsystem_dir(const char *name, struct dentry *d_events)
1009{1078{
@@ -1013,6 +1082,7 @@ event_subsystem_dir(const char *name, struct dentry *d_events) · 3 unmodified lines
1013 /* First see if we did not already create this dir */1082 /* First see if we did not already create this dir */
1014 list_for_each_entry(system, &event_subsystems, list) {1083 list_for_each_entry(system, &event_subsystems, list) {
1015 if (strcmp(system->name, name) == 0) {1084 if (strcmp(system->name, name) == 0) {
1085 __get_system(system);
1016 system->nr_events++;1086 system->nr_events++;
1017 return system->entry;1087 return system->entry;
1018 }1088 }
@@ -1035,6 +1105,7 @@ event_subsystem_dir(const char *name, struct dentry *d_events) · 16 unmodified lines
1035 }1105 }
1036 1106
1037 system->nr_events = 1;1107 system->nr_events = 1;
1108 system->ref_count = 1;
1038 system->name = kstrdup(name, GFP_KERNEL);1109 system->name = kstrdup(name, GFP_KERNEL);
1039 if (!system->name) {1110 if (!system->name) {
1040 debugfs_remove(system->entry);1111 debugfs_remove(system->entry);
@@ -1184,16 +1255,9 @@ static void remove_subsystem_dir(const char *name) · 143 unmodified lines
1184 list_for_each_entry(system, &event_subsystems, list) {1255 list_for_each_entry(system, &event_subsystems, list) {
1185 if (strcmp(system->name, name) == 0) {1256 if (strcmp(system->name, name) == 0) {
1186 if (!--system->nr_events) {1257 if (!--system->nr_events) {
1187 struct event_filter *filter = system->filter;
1188
1189 debugfs_remove_recursive(system->entry);1258 debugfs_remove_recursive(system->entry);
1190 list_del(&system->list);1259 list_del(&system->list);
1191 if (filter) {1260 __put_system(system);
1192 kfree(filter->filter_string);
1193 kfree(filter);
1194 }
1195 kfree(system->name);
1196 kfree(system);
1197 }1261 }
1198 break;1262 break;
1199 }1263 }
kernel/trace/trace_events_filter.c+6−0
@@ -1886,6 +1886,12 @@ int apply_subsystem_event_filter(struct event_subsystem *system, · 1885 unmodified lines
1886 1886
1887 mutex_lock(&event_mutex);1887 mutex_lock(&event_mutex);
1888 1888
1889 /* Make sure the system still has events */
1890 if (!system->nr_events) {
1891 err = -ENODEV;
1892 goto out_unlock;
1893 }
1894
1889 if (!strcmp(strstrip(filter_string), "0")) {1895 if (!strcmp(strstrip(filter_string), "0")) {
1890 filter_free_subsystem_preds(system);1896 filter_free_subsystem_preds(system);
1891 remove_filter_string(system->filter);1897 remove_filter_string(system->filter);