Messages in this thread Patch in this message | | | Date | Wed, 4 May 2022 12:09:03 +0900 | From | Masami Hiramatsu <> | Subject | Re: [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>
| |