lkml.org 
[lkml]   [2022]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[tip: x86/core] x86/ibt,ftrace: Search for __fentry__ location
    The following commit has been merged into the x86/core branch of tip:

    Commit-ID: aebfd12521d9c7d0b502cf6d06314cfbcdccfe3b
    Gitweb: https://git.kernel.org/tip/aebfd12521d9c7d0b502cf6d06314cfbcdccfe3b
    Author: Peter Zijlstra <peterz@infradead.org>
    AuthorDate: Tue, 08 Mar 2022 16:30:29 +01:00
    Committer: Peter Zijlstra <peterz@infradead.org>
    CommitterDate: Tue, 15 Mar 2022 10:32:37 +01:00

    x86/ibt,ftrace: Search for __fentry__ location

    Currently a lot of ftrace code assumes __fentry__ is at sym+0. However
    with Intel IBT enabled the first instruction of a function will most
    likely be ENDBR.

    Change ftrace_location() to not only return the __fentry__ location
    when called for the __fentry__ location, but also when called for the
    sym+0 location.

    Then audit/update all callsites of this function to consistently use
    these new semantics.

    Suggested-by: Steven Rostedt <rostedt@goodmis.org>
    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
    Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
    Link: https://lore.kernel.org/r/20220308154318.227581603@infradead.org
    ---
    arch/x86/kernel/kprobes/core.c | 11 +-------
    kernel/bpf/trampoline.c | 20 ++------------
    kernel/kprobes.c | 8 +-----
    kernel/trace/ftrace.c | 48 +++++++++++++++++++++++++++------
    4 files changed, 48 insertions(+), 39 deletions(-)

    diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
    index 6290712..4d8086a 100644
    --- a/arch/x86/kernel/kprobes/core.c
    +++ b/arch/x86/kernel/kprobes/core.c
    @@ -193,17 +193,10 @@ static unsigned long
    __recover_probed_insn(kprobe_opcode_t *buf, unsigned long addr)
    {
    struct kprobe *kp;
    - unsigned long faddr;
    + bool faddr;

    kp = get_kprobe((void *)addr);
    - faddr = ftrace_location(addr);
    - /*
    - * Addresses inside the ftrace location are refused by
    - * arch_check_ftrace_location(). Something went terribly wrong
    - * if such an address is checked here.
    - */
    - if (WARN_ON(faddr && faddr != addr))
    - return 0UL;
    + faddr = ftrace_location(addr) == addr;
    /*
    * Use the current code if it is not modified by Kprobe
    * and it cannot be modified by ftrace.
    diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
    index 5e7edf9..455f1b4 100644
    --- a/kernel/bpf/trampoline.c
    +++ b/kernel/bpf/trampoline.c
    @@ -117,18 +117,6 @@ static void bpf_trampoline_module_put(struct bpf_trampoline *tr)
    tr->mod = NULL;
    }

    -static int is_ftrace_location(void *ip)
    -{
    - long addr;
    -
    - addr = ftrace_location((long)ip);
    - if (!addr)
    - return 0;
    - if (WARN_ON_ONCE(addr != (long)ip))
    - return -EFAULT;
    - return 1;
    -}
    -
    static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)
    {
    void *ip = tr->func.addr;
    @@ -160,12 +148,12 @@ static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_ad
    static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
    {
    void *ip = tr->func.addr;
    + unsigned long faddr;
    int ret;

    - ret = is_ftrace_location(ip);
    - if (ret < 0)
    - return ret;
    - tr->func.ftrace_managed = ret;
    + faddr = ftrace_location((unsigned long)ip);
    + if (faddr)
    + tr->func.ftrace_managed = true;

    if (bpf_trampoline_module_get(tr))
    return -ENOENT;
    diff --git a/kernel/kprobes.c b/kernel/kprobes.c
    index 94cab8c..6d1e11c 100644
    --- a/kernel/kprobes.c
    +++ b/kernel/kprobes.c
    @@ -1562,14 +1562,10 @@ static inline int warn_kprobe_rereg(struct kprobe *p)

    static int check_ftrace_location(struct kprobe *p)
    {
    - unsigned long ftrace_addr;
    + unsigned long addr = (unsigned long)p->addr;

    - ftrace_addr = ftrace_location((unsigned long)p->addr);
    - if (ftrace_addr) {
    + if (ftrace_location(addr) == addr) {
    #ifdef CONFIG_KPROBES_ON_FTRACE
    - /* Given address is not on the instruction boundary */
    - if ((unsigned long)p->addr != ftrace_addr)
    - return -EILSEQ;
    p->flags |= KPROBE_FLAG_FTRACE;
    #else /* !CONFIG_KPROBES_ON_FTRACE */
    return -EINVAL;
    diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
    index 6105b70..2ae6fb9 100644
    --- a/kernel/trace/ftrace.c
    +++ b/kernel/trace/ftrace.c
    @@ -1568,17 +1568,34 @@ unsigned long ftrace_location_range(unsigned long start, unsigned long end)
    }

    /**
    - * ftrace_location - return true if the ip giving is a traced location
    + * ftrace_location - return the ftrace location
    * @ip: the instruction pointer to check
    *
    - * Returns rec->ip if @ip given is a pointer to a ftrace location.
    - * That is, the instruction that is either a NOP or call to
    - * the function tracer. It checks the ftrace internal tables to
    - * determine if the address belongs or not.
    + * If @ip matches the ftrace location, return @ip.
    + * If @ip matches sym+0, return sym's ftrace location.
    + * Otherwise, return 0.
    */
    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;
    +
    + /* map sym+0 to __fentry__ */
    + if (!offset)
    + rec = lookup_rec(ip, ip + size - 1);
    + }
    +
    + if (rec)
    + return rec->ip;
    +
    +out:
    + return 0;
    }

    /**
    @@ -4962,7 +4979,8 @@ ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove)
    {
    struct ftrace_func_entry *entry;

    - if (!ftrace_location(ip))
    + ip = ftrace_location(ip);
    + if (!ip)
    return -EINVAL;

    if (remove) {
    @@ -5110,11 +5128,16 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
    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;

    @@ -5222,6 +5245,10 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)

    mutex_lock(&direct_mutex);

    + ip = ftrace_location(ip);
    + if (!ip)
    + goto out_unlock;
    +
    entry = find_direct_entry(&ip, NULL);
    if (!entry)
    goto out_unlock;
    @@ -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;
    +
    entry = find_direct_entry(&ip, &rec);
    if (!entry)
    goto out_unlock;
    \
     
     \ /
      Last update: 2022-03-15 11:47    [W:7.841 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site