From: Oleg Nesterov <oleg@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>,
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Alexander Z Lam <azl@google.com>,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
David Sharp <dhsharp@google.com>,
Frederic Weisbecker <fweisbec@gmail.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
Vaibhav Nagarnaik <vnagarnaik@google.com>,
"zhangwei(Jovi)" <jovi.zhangwei@huawei.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH 2/6] tracing: Change event_enable/disable_read() to verify i_private != NULL
Date: Tue, 23 Jul 2013 22:59:13 +0200 [thread overview]
Message-ID: <20130723205913.GA9058@redhat.com> (raw)
In-Reply-To: <20130723205854.GA9036@redhat.com>
tracing_open_generic_file() is racy, ftrace_event_file can be
already freed by rmdir or trace_remove_event_call().
Change event_enable_read() and event_disable_read() to read and
verify "file = i_private" under event_mutex.
This fixes nothing, but now we can change debugfs_remove("enable")
callers to nullify ->i_private and fix the the problem.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/trace/trace_events.c | 32 ++++++++++++++++++++------------
1 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 77990c4..821768e 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -684,19 +684,28 @@ static ssize_t
event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
loff_t *ppos)
{
- struct ftrace_event_file *file = filp->private_data;
+ struct ftrace_event_file *file;
+ unsigned long flags;
char buf[4] = "0";
- if (file->flags & FTRACE_EVENT_FL_ENABLED &&
- !(file->flags & FTRACE_EVENT_FL_SOFT_DISABLED))
+ mutex_lock(&event_mutex);
+ file = event_file_data(filp);
+ if (likely(file))
+ flags = file->flags;
+ mutex_unlock(&event_mutex);
+
+ if (!file)
+ return -ENODEV;
+
+ if (flags & FTRACE_EVENT_FL_ENABLED &&
+ !(flags & FTRACE_EVENT_FL_SOFT_DISABLED))
strcpy(buf, "1");
- if (file->flags & FTRACE_EVENT_FL_SOFT_DISABLED ||
- file->flags & FTRACE_EVENT_FL_SOFT_MODE)
+ if (flags & FTRACE_EVENT_FL_SOFT_DISABLED ||
+ flags & FTRACE_EVENT_FL_SOFT_MODE)
strcat(buf, "*");
strcat(buf, "\n");
-
return simple_read_from_buffer(ubuf, cnt, ppos, buf, strlen(buf));
}
@@ -704,13 +713,10 @@ static ssize_t
event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
loff_t *ppos)
{
- struct ftrace_event_file *file = filp->private_data;
+ struct ftrace_event_file *file;
unsigned long val;
int ret;
- if (!file)
- return -EINVAL;
-
ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
if (ret)
return ret;
@@ -722,8 +728,11 @@ event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
switch (val) {
case 0:
case 1:
+ ret = -ENODEV;
mutex_lock(&event_mutex);
- ret = ftrace_event_enable_disable(file, val);
+ file = event_file_data(filp);
+ if (likely(file))
+ ret = ftrace_event_enable_disable(file, val);
mutex_unlock(&event_mutex);
break;
@@ -732,7 +741,6 @@ event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
}
*ppos += cnt;
-
return ret ? ret : cnt;
}
--
1.5.5.1
next prev parent reply other threads:[~2013-07-23 21:04 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-23 20:58 [PATCH 0/6] tracing: open/delete fixes Oleg Nesterov
2013-07-23 20:59 ` [PATCH 1/6] tracing: Turn event/id->i_private into call->event.type Oleg Nesterov
2013-07-24 20:13 ` Steven Rostedt
2013-07-25 14:26 ` Oleg Nesterov
2013-07-23 20:59 ` Oleg Nesterov [this message]
2013-07-24 19:37 ` [PATCH 2/6] tracing: Change event_enable/disable_read() to verify i_private != NULL Steven Rostedt
2013-07-23 20:59 ` [PATCH 3/6] tracing: Change event_filter_read/write " Oleg Nesterov
2013-07-24 19:52 ` Steven Rostedt
2013-07-23 20:59 ` [PATCH 4/6] tracing: Change f_start() to take event_mutex and " Oleg Nesterov
2013-07-23 20:59 ` [PATCH 5/6] tracing: Introduce remove_event_file_dir() Oleg Nesterov
2013-07-24 20:01 ` Steven Rostedt
2013-07-23 20:59 ` [PATCH 6/6] tracing: Change remove_event_file_dir() to clear "d_subdirs"->i_private Oleg Nesterov
2013-07-24 20:08 ` Steven Rostedt
2013-07-25 14:18 ` Oleg Nesterov
2013-07-24 18:46 ` [PATCH 0/6] tracing: open/delete fixes Oleg Nesterov
2013-07-25 16:56 ` Oleg Nesterov
2013-07-25 19:27 ` PATCH? debugfs_remove_recursive() must not rely on list_empty(d_subdirs) Oleg Nesterov
2013-07-25 20:04 ` Oleg Nesterov
2013-07-25 23:43 ` Greg Kroah-Hartman
2013-07-26 15:11 ` [PATCH 0/1] debugfs: " Oleg Nesterov
2013-07-26 15:12 ` Oleg Nesterov
2013-07-26 15:14 ` Oleg Nesterov
2013-07-26 15:12 ` [PATCH 1/1] " Oleg Nesterov
2013-07-26 17:38 ` Greg Kroah-Hartman
2013-07-26 18:40 ` Steven Rostedt
2013-07-26 15:30 ` [PATCH 0/1] " Steven Rostedt
2013-07-26 16:28 ` Greg Kroah-Hartman
2013-07-26 17:38 ` Greg Kroah-Hartman
2013-07-26 10:24 ` Re: PATCH? " Masami Hiramatsu
2013-07-26 14:49 ` Oleg Nesterov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130723205913.GA9058@redhat.com \
--to=oleg@redhat.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=azl@google.com \
--cc=dhsharp@google.com \
--cc=fweisbec@gmail.com \
--cc=jovi.zhangwei@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=masami.hiramatsu.pt@hitachi.com \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
--cc=srikar@linux.vnet.ibm.com \
--cc=vnagarnaik@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox