diff options
author | Oleg Nesterov <oleg@redhat.com> | 2013-07-26 13:25:40 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2013-08-29 12:47:33 -0400 |
commit | 70c91fb9a74e7937ef76a542a86c1551d5df4281 (patch) | |
tree | 16cec9beb88a2f535a4cac8ed8bbe29831fba577 /kernel | |
parent | df89bf77ca930f69ba07fd459f735ec3cac69f8f (diff) |
tracing: Change event_filter_read/write to verify i_private != NULL
commit e2912b091c26b8ea95e5e00a43a7ac620f6c94a6 upstream.
event_filter_read/write() are racy, ftrace_event_call can be already
freed by trace_remove_event_call() callers.
1. Shift mutex_lock(event_mutex) from print/apply_event_filter to
the callers.
2. Change the callers, event_filter_read() and event_filter_write()
to read i_private under this mutex and abort if it is NULL.
This fixes nothing, but now we can change debugfs_remove("filter")
callers to nullify ->i_private and fix the the problem.
Link: http://lkml.kernel.org/r/20130726172540.GA3619@redhat.com
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'kernel')
-rw-r--r-- | kernel/trace/trace_events.c | 26 | ||||
-rw-r--r-- | kernel/trace/trace_events_filter.c | 17 |
2 files changed, 25 insertions, 18 deletions
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index e8c445d1f190..285589354cb7 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c | |||
@@ -1002,21 +1002,28 @@ static ssize_t | |||
1002 | event_filter_read(struct file *filp, char __user *ubuf, size_t cnt, | 1002 | event_filter_read(struct file *filp, char __user *ubuf, size_t cnt, |
1003 | loff_t *ppos) | 1003 | loff_t *ppos) |
1004 | { | 1004 | { |
1005 | struct ftrace_event_call *call = filp->private_data; | 1005 | struct ftrace_event_call *call; |
1006 | struct trace_seq *s; | 1006 | struct trace_seq *s; |
1007 | int r; | 1007 | int r = -ENODEV; |
1008 | 1008 | ||
1009 | if (*ppos) | 1009 | if (*ppos) |
1010 | return 0; | 1010 | return 0; |
1011 | 1011 | ||
1012 | s = kmalloc(sizeof(*s), GFP_KERNEL); | 1012 | s = kmalloc(sizeof(*s), GFP_KERNEL); |
1013 | |||
1013 | if (!s) | 1014 | if (!s) |
1014 | return -ENOMEM; | 1015 | return -ENOMEM; |
1015 | 1016 | ||
1016 | trace_seq_init(s); | 1017 | trace_seq_init(s); |
1017 | 1018 | ||
1018 | print_event_filter(call, s); | 1019 | mutex_lock(&event_mutex); |
1019 | r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->len); | 1020 | call = event_file_data(filp); |
1021 | if (call) | ||
1022 | print_event_filter(call, s); | ||
1023 | mutex_unlock(&event_mutex); | ||
1024 | |||
1025 | if (call) | ||
1026 | r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->len); | ||
1020 | 1027 | ||
1021 | kfree(s); | 1028 | kfree(s); |
1022 | 1029 | ||
@@ -1027,9 +1034,9 @@ static ssize_t | |||
1027 | event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt, | 1034 | event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt, |
1028 | loff_t *ppos) | 1035 | loff_t *ppos) |
1029 | { | 1036 | { |
1030 | struct ftrace_event_call *call = filp->private_data; | 1037 | struct ftrace_event_call *call; |
1031 | char *buf; | 1038 | char *buf; |
1032 | int err; | 1039 | int err = -ENODEV; |
1033 | 1040 | ||
1034 | if (cnt >= PAGE_SIZE) | 1041 | if (cnt >= PAGE_SIZE) |
1035 | return -EINVAL; | 1042 | return -EINVAL; |
@@ -1044,7 +1051,12 @@ event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt, | |||
1044 | } | 1051 | } |
1045 | buf[cnt] = '\0'; | 1052 | buf[cnt] = '\0'; |
1046 | 1053 | ||
1047 | err = apply_event_filter(call, buf); | 1054 | mutex_lock(&event_mutex); |
1055 | call = event_file_data(filp); | ||
1056 | if (call) | ||
1057 | err = apply_event_filter(call, buf); | ||
1058 | mutex_unlock(&event_mutex); | ||
1059 | |||
1048 | free_page((unsigned long) buf); | 1060 | free_page((unsigned long) buf); |
1049 | if (err < 0) | 1061 | if (err < 0) |
1050 | return err; | 1062 | return err; |
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c index e1b653f7e1ca..0a1edc694d67 100644 --- a/kernel/trace/trace_events_filter.c +++ b/kernel/trace/trace_events_filter.c | |||
@@ -631,17 +631,15 @@ static void append_filter_err(struct filter_parse_state *ps, | |||
631 | free_page((unsigned long) buf); | 631 | free_page((unsigned long) buf); |
632 | } | 632 | } |
633 | 633 | ||
634 | /* caller must hold event_mutex */ | ||
634 | void print_event_filter(struct ftrace_event_call *call, struct trace_seq *s) | 635 | void print_event_filter(struct ftrace_event_call *call, struct trace_seq *s) |
635 | { | 636 | { |
636 | struct event_filter *filter; | 637 | struct event_filter *filter = call->filter; |
637 | 638 | ||
638 | mutex_lock(&event_mutex); | ||
639 | filter = call->filter; | ||
640 | if (filter && filter->filter_string) | 639 | if (filter && filter->filter_string) |
641 | trace_seq_printf(s, "%s\n", filter->filter_string); | 640 | trace_seq_printf(s, "%s\n", filter->filter_string); |
642 | else | 641 | else |
643 | trace_seq_printf(s, "none\n"); | 642 | trace_seq_printf(s, "none\n"); |
644 | mutex_unlock(&event_mutex); | ||
645 | } | 643 | } |
646 | 644 | ||
647 | void print_subsystem_event_filter(struct event_subsystem *system, | 645 | void print_subsystem_event_filter(struct event_subsystem *system, |
@@ -1835,23 +1833,22 @@ static int create_system_filter(struct event_subsystem *system, | |||
1835 | return err; | 1833 | return err; |
1836 | } | 1834 | } |
1837 | 1835 | ||
1836 | /* caller must hold event_mutex */ | ||
1838 | int apply_event_filter(struct ftrace_event_call *call, char *filter_string) | 1837 | int apply_event_filter(struct ftrace_event_call *call, char *filter_string) |
1839 | { | 1838 | { |
1840 | struct event_filter *filter; | 1839 | struct event_filter *filter; |
1841 | int err = 0; | 1840 | int err; |
1842 | |||
1843 | mutex_lock(&event_mutex); | ||
1844 | 1841 | ||
1845 | if (!strcmp(strstrip(filter_string), "0")) { | 1842 | if (!strcmp(strstrip(filter_string), "0")) { |
1846 | filter_disable(call); | 1843 | filter_disable(call); |
1847 | filter = call->filter; | 1844 | filter = call->filter; |
1848 | if (!filter) | 1845 | if (!filter) |
1849 | goto out_unlock; | 1846 | return 0; |
1850 | RCU_INIT_POINTER(call->filter, NULL); | 1847 | RCU_INIT_POINTER(call->filter, NULL); |
1851 | /* Make sure the filter is not being used */ | 1848 | /* Make sure the filter is not being used */ |
1852 | synchronize_sched(); | 1849 | synchronize_sched(); |
1853 | __free_filter(filter); | 1850 | __free_filter(filter); |
1854 | goto out_unlock; | 1851 | return 0; |
1855 | } | 1852 | } |
1856 | 1853 | ||
1857 | err = create_filter(call, filter_string, true, &filter); | 1854 | err = create_filter(call, filter_string, true, &filter); |
@@ -1878,8 +1875,6 @@ int apply_event_filter(struct ftrace_event_call *call, char *filter_string) | |||
1878 | __free_filter(tmp); | 1875 | __free_filter(tmp); |
1879 | } | 1876 | } |
1880 | } | 1877 | } |
1881 | out_unlock: | ||
1882 | mutex_unlock(&event_mutex); | ||
1883 | 1878 | ||
1884 | return err; | 1879 | return err; |
1885 | } | 1880 | } |