[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YitpVCIllFrnakpL@hirez.programming.kicks-ass.net>
Date: Fri, 11 Mar 2022 16:23:00 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>, x86@...nel.org,
joao@...rdrivepizza.com, hjl.tools@...il.com, jpoimboe@...hat.com,
andrew.cooper3@...rix.com, linux-kernel@...r.kernel.org,
ndesaulniers@...gle.com, keescook@...omium.org,
samitolvanen@...gle.com, mark.rutland@....com,
alyssa.milburn@...el.com, mbenes@...e.cz, mhiramat@...nel.org,
daniel@...earbox.net, andrii@...nel.org, bpf@...r.kernel.org
Subject: Re: [PATCH v4 00/45] x86: Kernel IBT
On Thu, Mar 10, 2022 at 09:37:31AM -0500, Steven Rostedt wrote:
> On Thu, 10 Mar 2022 14:47:18 +0100
> Peter Zijlstra <peterz@...radead.org> wrote:
>
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index acb50fb7ed2d..2d86d3c09d64 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -5354,6 +5381,11 @@ int modify_ftrace_direct(unsigned long ip,
> > mutex_lock(&direct_mutex);
> >
> > mutex_lock(&ftrace_lock);
> > +
> > + ip = ftrace_location(ip);
> > + if (!ip)
> > + goto out_unlock;
> > +
>
> Perhaps this should go into find_direct_entry() instead, as I think you are
> adding it before all the find_direct_entry() callers.
Something like so then?
Index: linux-2.6/kernel/trace/ftrace.c
===================================================================
--- linux-2.6.orig/kernel/trace/ftrace.c
+++ linux-2.6/kernel/trace/ftrace.c
@@ -1575,7 +1575,7 @@ unsigned long ftrace_location_range(unsi
* If @ip matches sym+0, return sym's ftrace location.
* Otherwise, return 0.
*/
-unsigned long ftrace_location(unsigned long ip)
+unsigned long __ftrace_location(unsigned long ip, struct dyn_ftrace **recp)
{
struct dyn_ftrace *rec;
unsigned long offset;
@@ -1591,13 +1591,22 @@ unsigned long ftrace_location(unsigned l
rec = lookup_rec(ip, ip + size - 1);
}
- if (rec)
+ if (rec) {
+ if (recp)
+ *recp = rec;
+
return rec->ip;
+ }
out:
return 0;
}
+unsigned long ftrace_location(unsigned long ip)
+{
+ return __ftrace_location(ip, NULL);
+}
+
/**
* ftrace_text_reserved - return true if range contains an ftrace location
* @start: start of range to search
@@ -2392,6 +2401,30 @@ static struct ftrace_hash *direct_functi
static DEFINE_MUTEX(direct_mutex);
int ftrace_direct_func_count;
+static struct ftrace_func_entry *
+find_direct_entry(unsigned long *ip, struct dyn_ftrace **recp, bool warn)
+{
+ struct ftrace_func_entry *entry;
+ struct dyn_ftrace *rec = NULL;
+
+ *ip = __ftrace_location(*ip, &rec);
+ if (!*ip)
+ return NULL;
+
+ if (recp)
+ *recp = rec;
+
+ entry = __ftrace_lookup_ip(direct_functions, *ip);
+ if (!entry) {
+ WARN_ON(rec->flags & FTRACE_FL_DIRECT);
+ return NULL;
+ }
+
+ WARN_ON(warn && !(rec->flags & FTRACE_FL_DIRECT));
+
+ return entry;
+}
+
/*
* Search the direct_functions hash to see if the given instruction pointer
* has a direct caller attached to it.
@@ -2400,7 +2433,7 @@ unsigned long ftrace_find_rec_direct(uns
{
struct ftrace_func_entry *entry;
- entry = __ftrace_lookup_ip(direct_functions, ip);
+ entry = find_direct_entry(&ip, NULL, false);
if (!entry)
return 0;
@@ -5127,40 +5160,19 @@ int register_ftrace_direct(unsigned long
struct ftrace_direct_func *direct;
struct ftrace_func_entry *entry;
struct ftrace_hash *free_hash = NULL;
- struct dyn_ftrace *rec;
+ struct dyn_ftrace *rec = NULL;
int ret = -ENODEV;
mutex_lock(&direct_mutex);
- ip = ftrace_location(ip);
- if (!ip)
+ entry = find_direct_entry(&ip, &rec, true);
+ if (!ip || !rec)
goto out_unlock;
- /* See if there's a direct function at @ip already */
ret = -EBUSY;
- if (ftrace_find_rec_direct(ip))
- goto out_unlock;
-
- ret = -ENODEV;
- rec = lookup_rec(ip, ip);
- if (!rec)
+ if (entry && entry->direct)
goto out_unlock;
- /*
- * Check if the rec says it has a direct call but we didn't
- * find one earlier?
- */
- if (WARN_ON(rec->flags & FTRACE_FL_DIRECT))
- goto out_unlock;
-
- /* Make sure the ip points to the exact record */
- if (ip != rec->ip) {
- ip = rec->ip;
- /* Need to check this ip for a direct. */
- if (ftrace_find_rec_direct(ip))
- goto out_unlock;
- }
-
ret = -ENOMEM;
direct = ftrace_find_direct_func(addr);
if (!direct) {
@@ -5209,33 +5221,6 @@ int register_ftrace_direct(unsigned long
}
EXPORT_SYMBOL_GPL(register_ftrace_direct);
-static struct ftrace_func_entry *find_direct_entry(unsigned long *ip,
- struct dyn_ftrace **recp)
-{
- struct ftrace_func_entry *entry;
- struct dyn_ftrace *rec;
-
- rec = lookup_rec(*ip, *ip);
- if (!rec)
- return NULL;
-
- entry = __ftrace_lookup_ip(direct_functions, rec->ip);
- if (!entry) {
- WARN_ON(rec->flags & FTRACE_FL_DIRECT);
- return NULL;
- }
-
- WARN_ON(!(rec->flags & FTRACE_FL_DIRECT));
-
- /* Passed in ip just needs to be on the call site */
- *ip = rec->ip;
-
- if (recp)
- *recp = rec;
-
- return entry;
-}
-
int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
{
struct ftrace_direct_func *direct;
@@ -5245,11 +5230,7 @@ int unregister_ftrace_direct(unsigned lo
mutex_lock(&direct_mutex);
- ip = ftrace_location(ip);
- if (!ip)
- goto out_unlock;
-
- entry = find_direct_entry(&ip, NULL);
+ entry = find_direct_entry(&ip, NULL, true);
if (!entry)
goto out_unlock;
@@ -5382,11 +5363,7 @@ int modify_ftrace_direct(unsigned long i
mutex_lock(&ftrace_lock);
- ip = ftrace_location(ip);
- if (!ip)
- goto out_unlock;
-
- entry = find_direct_entry(&ip, &rec);
+ entry = find_direct_entry(&ip, &rec, true);
if (!entry)
goto out_unlock;
Powered by blists - more mailing lists