From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>,
Andrew Morton <akpm@linux-foundation.org>,
Frederic Weisbecker <fweisbec@gmail.com>,
Li Zefan <lizf@cn.fujitsu.com>, Christoph Hellwig <hch@lst.de>
Subject: [PATCH 6/7] tracing: reset ring buffer when removing modules with events
Date: Wed, 06 May 2009 23:13:41 -0400 [thread overview]
Message-ID: <20090507031434.586509269@goodmis.org> (raw)
In-Reply-To: 20090507031335.815354104@goodmis.org
[-- Attachment #1: 0006-tracing-reset-ring-buffer-when-removing-modules-wit.patch --]
[-- Type: text/plain, Size: 3418 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Li Zefan found that there's a race using the event ids of events and
modules. When a module is loaded, an event id is incremented. We only
have 16 bits for event ids (65536) and there is a possible (but highly
unlikely) race that we could load and unload a module that registers
events so many times that the event id counter overflows.
When it overflows, it then restarts and goes looking for available
ids. An id is available if it was added by a module and released.
The race is if you have one module add an id, and then is removed.
Another module loaded can use that same event id. But if the old module
still had events in the ring buffer, the new module's call back would
get bogus data. At best (and most likely) the output would just be
garbage. But if the module for some reason used pointers (not recommended)
then this could potentially crash.
The safest thing to do is just reset the ring buffer if a module that
registered events is removed.
[ Impact: prevent unpredictable results of event id overflows ]
Reported-by: Li Zefan <lizf@cn.fujitsu.com>
LKML-Reference: <49FEAFD0.30106@cn.fujitsu.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace.c | 10 ++++++++++
kernel/trace/trace.h | 2 ++
kernel/trace/trace_events.c | 9 +++++++++
3 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4164a34..dd40d23 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -639,6 +639,16 @@ void tracing_reset_online_cpus(struct trace_array *tr)
tracing_reset(tr, cpu);
}
+void tracing_reset_current(int cpu)
+{
+ tracing_reset(&global_trace, cpu);
+}
+
+void tracing_reset_current_online_cpus(void)
+{
+ tracing_reset_online_cpus(&global_trace);
+}
+
#define SAVED_CMDLINES 128
#define NO_CMDLINE_MAP UINT_MAX
static unsigned map_pid_to_cmdline[PID_MAX_DEFAULT+1];
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 777c6c3..ba25793 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -409,6 +409,8 @@ int tracing_is_enabled(void);
void trace_wake_up(void);
void tracing_reset(struct trace_array *tr, int cpu);
void tracing_reset_online_cpus(struct trace_array *tr);
+void tracing_reset_current(int cpu);
+void tracing_reset_current_online_cpus(void);
int tracing_open_generic(struct inode *inode, struct file *filp);
struct dentry *trace_create_file(const char *name,
mode_t mode,
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 8d579ff..6d2c842 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -932,9 +932,11 @@ static void trace_module_remove_events(struct module *mod)
{
struct ftrace_module_file_ops *file_ops;
struct ftrace_event_call *call, *p;
+ bool found = false;
list_for_each_entry_safe(call, p, &ftrace_events, list) {
if (call->mod == mod) {
+ found = true;
if (call->enabled) {
call->enabled = 0;
call->unregfunc();
@@ -957,6 +959,13 @@ static void trace_module_remove_events(struct module *mod)
list_del(&file_ops->list);
kfree(file_ops);
}
+
+ /*
+ * It is safest to reset the ring buffer if the module being unloaded
+ * registered any events.
+ */
+ if (found)
+ tracing_reset_current_online_cpus();
}
static int trace_module_notify(struct notifier_block *self,
--
1.6.2.4
--
next prev parent reply other threads:[~2009-05-07 3:16 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-07 3:13 [PATCH 0/7] [GIT PULL] tracing/ring-buffer: more updates for tip Steven Rostedt
2009-05-07 3:13 ` [PATCH 1/7] ring-buffer: remove unneeded conditional in rb_reserve_next Steven Rostedt
2009-05-07 8:23 ` Ingo Molnar
2009-05-07 3:13 ` [PATCH 2/7] ring-buffer: check for failed allocation in ring buffer benchmark Steven Rostedt
2009-05-07 3:13 ` [PATCH 3/7] ring-buffer: make moving the tail page a separate function Steven Rostedt
2009-05-07 8:27 ` Ingo Molnar
2009-05-07 13:26 ` Steven Rostedt
2009-05-07 13:56 ` Ingo Molnar
2009-05-07 3:13 ` [PATCH 4/7] ring-buffer: change test to be more latency friendly Steven Rostedt
2009-05-07 8:31 ` Ingo Molnar
2009-05-07 8:34 ` Ingo Molnar
2009-05-07 13:51 ` Steven Rostedt
2009-05-07 3:13 ` [PATCH 5/7] tracing: update sample with TRACE_INCLUDE_FILE Steven Rostedt
2009-05-07 3:13 ` Steven Rostedt [this message]
2009-05-07 3:51 ` [PATCH 6/7] tracing: reset ring buffer when removing modules with events Li Zefan
2009-05-07 16:24 ` Frederic Weisbecker
2009-05-07 3:13 ` [PATCH 7/7] tracing: add hierarchical enabling of events Steven Rostedt
2009-05-07 3:51 ` Li Zefan
2009-05-07 13:21 ` Steven Rostedt
2009-05-08 1:11 ` Li Zefan
2009-05-08 1:23 ` Steven Rostedt
2009-05-08 1:24 ` Steven Rostedt
2009-05-07 16:28 ` Frederic Weisbecker
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=20090507031434.586509269@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=fweisbec@gmail.com \
--cc=hch@lst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=lizf@cn.fujitsu.com \
--cc=mingo@elte.hu \
/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