lkml.org 
[lkml]   [2022]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] kprobe: sync issue's on ftraced-kprobe.
On Mon,  2 May 2022 13:51:02 +0900
Levi Yun <ppbuk5246@gmail.com> wrote:

> In kprobe_ftrace_handler, it accesses get kporbe without kprobe_mutex
> held.
>
> This makes some of synchronizing issue when we use kprobe API in
> kernel-module.

NAK this, because get_kprobes() doesn't require the kprobe_mutex in
the preempt-disabled context. Please read the comment of get_kprobe().

/*
* This routine is called either:
* - under the 'kprobe_mutex' - during kprobe_[un]register().
* OR
* - with preemption disabled - from architecture specific code.
*/
struct kprobe *get_kprobe(void *addr)

Moreover, we can not use mutex inside kprobe handler because it runs
in the interrupt context.

Thank you,

>
> Below is what i experienced:
>
> CPU 0 CPU 1
> <...> <In module code>
> kprobe_ftrace_handler
> get_kprobe
> __this_cpu_write
> unregister_kprobe
> unload_module
> < kprobe memory gone>
> p->pre_handler <access invalid memory>
> page_fault
> kprobe_fault_handler
> (In here, kprobe memory gone,
> double page fault is happening inifinie).
>
> Signed-off-by: Levi Yun <ppbuk5246@gmail.com>
> ---
> arch/x86/kernel/kprobes/ftrace.c | 3 +++
> include/linux/kprobes.h | 2 ++
> kernel/kprobes.c | 2 +-
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/kprobes/ftrace.c b/arch/x86/kernel/kprobes/ftrace.c
> index dd2ec14adb77..76147ff6ed88 100644
> --- a/arch/x86/kernel/kprobes/ftrace.c
> +++ b/arch/x86/kernel/kprobes/ftrace.c
> @@ -25,6 +25,7 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> if (bit < 0)
> return;
>
> + mutex_lock(&kprobe_mutex);
> p = get_kprobe((kprobe_opcode_t *)ip);
> if (unlikely(!p) || kprobe_disabled(p))
> goto out;
> @@ -57,7 +58,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> */
> __this_cpu_write(current_kprobe, NULL);
> }
> +
> out:
> + mutex_unlock(&kprobe_mutex);
> ftrace_test_recursion_unlock(bit);
> }
> NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 157168769fc2..4a18147ff6d6 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -191,6 +191,8 @@ struct kprobe_blacklist_entry {
> DECLARE_PER_CPU(struct kprobe *, current_kprobe);
> DECLARE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>
> +extern struct mutex kprobe_mutex;
> +
> extern void kprobe_busy_begin(void);
> extern void kprobe_busy_end(void);
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index dd58c0be9ce2..b65f055b6fa2 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -64,7 +64,7 @@ static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
> static bool kprobes_all_disarmed;
>
> /* This protects 'kprobe_table' and 'optimizing_list' */
> -static DEFINE_MUTEX(kprobe_mutex);
> +DEFINE_MUTEX(kprobe_mutex);
> static DEFINE_PER_CPU(struct kprobe *, kprobe_instance);
>
> kprobe_opcode_t * __weak kprobe_lookup_name(const char *name,
> --
> 2.35.1
>


--
Masami Hiramatsu <mhiramat@kernel.org>

\
 
 \ /
  Last update: 2022-05-04 05:10    [W:0.081 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site