lkml.org 
[lkml]   [2022]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v2 12/39] x86/ibt,ftrace: Search for __fentry__ location
    Peter Zijlstra wrote:
    > Have ftrace_location() search the symbol for the __fentry__ location
    > when it isn't at func+0 and use this for {,un}register_ftrace_direct().
    >
    > This avoids a whole bunch of assumptions about __fentry__ being at
    > func+0.
    >
    > Suggested-by: Steven Rostedt <rostedt@goodmis.org>
    > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    > ---
    > kernel/trace/ftrace.c | 30 ++++++++++++++++++++++++++++--
    > 1 file changed, 28 insertions(+), 2 deletions(-)
    >
    > --- a/kernel/trace/ftrace.c
    > +++ b/kernel/trace/ftrace.c
    > @@ -1578,7 +1578,24 @@ unsigned long ftrace_location_range(unsi
    > */
    > unsigned long ftrace_location(unsigned long ip)
    > {
    > - return ftrace_location_range(ip, ip);
    > + struct dyn_ftrace *rec;
    > + unsigned long offset;
    > + unsigned long size;
    > +
    > + rec = lookup_rec(ip, ip);
    > + if (!rec) {
    > + if (!kallsyms_lookup_size_offset(ip, &size, &offset))
    > + goto out;
    > +
    > + if (!offset)
    > + rec = lookup_rec(ip - offset, (ip - offset) + size);
    > + }
    > +
    > + if (rec)
    > + return rec->ip;
    > +
    > +out:
    > + return 0;
    > }
    >
    > /**
    > @@ -5110,11 +5127,16 @@ int register_ftrace_direct(unsigned long
    > struct ftrace_func_entry *entry;
    > struct ftrace_hash *free_hash = NULL;
    > struct dyn_ftrace *rec;
    > - int ret = -EBUSY;
    > + int ret = -ENODEV;
    >
    > mutex_lock(&direct_mutex);
    >
    > + ip = ftrace_location(ip);
    > + if (!ip)
    > + goto out_unlock;
    > +
    > /* See if there's a direct function at @ip already */
    > + ret = -EBUSY;
    > if (ftrace_find_rec_direct(ip))
    > goto out_unlock;

    I think some of the validation at this point can be removed (diff below).

    >
    > @@ -5222,6 +5244,10 @@ 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);
    > if (!entry)
    > goto out_unlock;

    We should also update modify_ftrace_direct(). An incremental diff below.


    - Naveen


    ---
    diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
    index 65d7553668ca3d..17ce4751a2051a 100644
    --- a/kernel/trace/ftrace.c
    +++ b/kernel/trace/ftrace.c
    @@ -5126,7 +5126,6 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
    struct ftrace_direct_func *direct;
    struct ftrace_func_entry *entry;
    struct ftrace_hash *free_hash = NULL;
    - struct dyn_ftrace *rec;
    int ret = -ENODEV;

    mutex_lock(&direct_mutex);
    @@ -5140,26 +5139,6 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
    if (ftrace_find_rec_direct(ip))
    goto out_unlock;

    - ret = -ENODEV;
    - rec = lookup_rec(ip, ip);
    - if (!rec)
    - 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) {
    @@ -5380,6 +5359,10 @@ int modify_ftrace_direct(unsigned long ip,
    mutex_lock(&direct_mutex);

    mutex_lock(&ftrace_lock);
    + ip = ftrace_location(ip);
    + if (!ip)
    + goto out_unlock;
    +
    entry = find_direct_entry(&ip, &rec);
    if (!entry)
    goto out_unlock;
    --
    2.35.1
    \
     
     \ /
      Last update: 2022-03-02 17:27    [W:5.182 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site