From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Namhyung Kim <namhyung@kernel.org>
Subject: [for-next][PATCH 7/8] ftrace: Do not hold references of ftrace_graph_{notrace_}hash out of graph_lock
Date: Fri, 03 Feb 2017 08:40:40 -0500 [thread overview]
Message-ID: <20170203134137.946282923@goodmis.org> (raw)
In-Reply-To: 20170203134033.087760237@goodmis.org
[-- Attachment #1: 0007-ftrace-Do-not-hold-references-of-ftrace_graph_-notra.patch --]
[-- Type: text/plain, Size: 3196 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The hashs ftrace_graph_hash and ftrace_graph_notrace_hash are modified
within the graph_lock being held. Holding a pointer to them and passing them
along can lead to a use of a stale pointer (fgd->hash). Move assigning the
pointer and its use to within the holding of the lock. Note, it's an
rcu_sched protected data, and other instances of referencing them are done
with preemption disabled. But the file manipuation code must be protected by
the lock.
The fgd->hash pointer is set to NULL when the lock is being released.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0233c8cb45f4..b3a4896ef78a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4616,6 +4616,13 @@ static void *g_start(struct seq_file *m, loff_t *pos)
mutex_lock(&graph_lock);
+ if (fgd->type == GRAPH_FILTER_FUNCTION)
+ fgd->hash = rcu_dereference_protected(ftrace_graph_hash,
+ lockdep_is_held(&graph_lock));
+ else
+ fgd->hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
+ lockdep_is_held(&graph_lock));
+
/* Nothing, tell g_show to print all functions are enabled */
if (ftrace_hash_empty(fgd->hash) && !*pos)
return FTRACE_GRAPH_EMPTY;
@@ -4695,6 +4702,14 @@ __ftrace_graph_open(struct inode *inode, struct file *file,
out:
fgd->new_hash = new_hash;
+
+ /*
+ * All uses of fgd->hash must be taken with the graph_lock
+ * held. The graph_lock is going to be released, so force
+ * fgd->hash to be reinitialized when it is taken again.
+ */
+ fgd->hash = NULL;
+
return ret;
}
@@ -4713,7 +4728,8 @@ ftrace_graph_open(struct inode *inode, struct file *file)
mutex_lock(&graph_lock);
- fgd->hash = ftrace_graph_hash;
+ fgd->hash = rcu_dereference_protected(ftrace_graph_hash,
+ lockdep_is_held(&graph_lock));
fgd->type = GRAPH_FILTER_FUNCTION;
fgd->seq_ops = &ftrace_graph_seq_ops;
@@ -4740,7 +4756,8 @@ ftrace_graph_notrace_open(struct inode *inode, struct file *file)
mutex_lock(&graph_lock);
- fgd->hash = ftrace_graph_notrace_hash;
+ fgd->hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
+ lockdep_is_held(&graph_lock));
fgd->type = GRAPH_FILTER_NOTRACE;
fgd->seq_ops = &ftrace_graph_seq_ops;
@@ -4859,17 +4876,18 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
ret = ftrace_graph_set_hash(fgd->new_hash,
parser.buffer);
- old_hash = fgd->hash;
new_hash = __ftrace_hash_move(fgd->new_hash);
if (!new_hash)
ret = -ENOMEM;
if (fgd->type == GRAPH_FILTER_FUNCTION) {
+ old_hash = rcu_dereference_protected(ftrace_graph_hash,
+ lockdep_is_held(&graph_lock));
rcu_assign_pointer(ftrace_graph_hash, new_hash);
- fgd->hash = ftrace_graph_hash;
} else {
+ old_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
+ lockdep_is_held(&graph_lock));
rcu_assign_pointer(ftrace_graph_notrace_hash, new_hash);
- fgd->hash = ftrace_graph_notrace_hash;
}
mutex_unlock(&graph_lock);
--
2.10.2
next prev parent reply other threads:[~2017-02-03 13:42 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-03 13:40 [for-next][PATCH 0/8] tracing: Clean up hash logic for set_graph_function Steven Rostedt
2017-02-03 13:40 ` [for-next][PATCH 1/8] tracing: Add ftrace_hash_key() helper function Steven Rostedt
2017-02-03 13:40 ` [for-next][PATCH 2/8] ftrace: Create a slight optimization on searching the ftrace_hash Steven Rostedt
2017-02-03 14:26 ` Namhyung Kim
2017-02-03 14:57 ` Steven Rostedt
2017-02-03 13:40 ` [for-next][PATCH 3/8] ftrace: Replace (void *)1 with a meaningful macro name FTRACE_GRAPH_EMPTY Steven Rostedt
2017-02-03 13:40 ` [for-next][PATCH 4/8] ftrace: Reset fgd->hash in ftrace_graph_write() Steven Rostedt
2017-02-03 14:49 ` Namhyung Kim
2017-02-03 14:57 ` Steven Rostedt
2017-02-03 13:40 ` [for-next][PATCH 5/8] ftrace: Have set_graph_functions handle write with RDWR Steven Rostedt
2017-02-03 13:40 ` [for-next][PATCH 6/8] tracing: Reset parser->buffer to allow multiple "puts" Steven Rostedt
2017-02-03 13:40 ` Steven Rostedt [this message]
2017-02-03 13:40 ` [for-next][PATCH 8/8] ftrace: Have set_graph_function handle multiple functions in one write Steven Rostedt
2017-02-03 15:14 ` [for-next][PATCH 0/8] tracing: Clean up hash logic for set_graph_function Namhyung Kim
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=20170203134137.946282923@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.